Skip to content

Commit 24bae29

Browse files
saw-jansaw-jan
authored
fix: handle op projects whose parent project is unknown (#985)
* fix: hanlding undisclosed and non-existing project parent Signed-off-by: Saw-jan <saw.jan.grg3e@gmail.com> * test: fix vue unit tests Signed-off-by: Saw-jan <saw.jan.grg3e@gmail.com> * test: add vue unit tests Signed-off-by: Saw-jan <saw.jan.grg3e@gmail.com> * chore: add changelog entry Signed-off-by: Saw-jan <saw.jan.grg3e@gmail.com> --------- Signed-off-by: Saw-jan <saw.jan.grg3e@gmail.com>
1 parent 379b708 commit 24bae29

5 files changed

Lines changed: 123 additions & 64 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
1313

1414
## [Unreleased]
1515

16+
### Fixed
17+
18+
- Fix: Handle projects whose parent project is unknown [#985](https://github.com/nextcloud/integration_openproject/pull/985)
19+
1620
### Added
1721

1822
### Changed

‎src/components/tab/SearchInput.vue‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,8 @@
4747
</NcButton>
4848
</div>
4949
<CreateWorkPackageModal
50-
v-if="!isSmartPicker"
50+
v-if="!isSmartPicker && isCreateWorkpackageModalVisible"
5151
ref="testRef"
52-
:show-modal="isCreateWorkpackageModalVisible"
5352
data-test-id="create-workpackage-modal"
5453
@create-work-package="onCreateWorkPackageEvent"
5554
@close-create-work-package-modal="onCloseCreateWorkPackageModalEvent" />

‎src/views/CreateWorkPackageModal.vue‎

Lines changed: 50 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
- SPDX-License-Identifier: AGPL-3.0-or-later
55
-->
66
<template>
7-
<NcModal v-if="openModal"
7+
<NcModal
88
class="create-workpackage-modal"
99
:can-close="true"
1010
:out-transition="true"
@@ -233,12 +233,6 @@ export default {
233233
NcModal,
234234
NcTextField,
235235
},
236-
props: {
237-
showModal: {
238-
type: Boolean,
239-
default: false,
240-
},
241-
},
242236
data: () => ({
243237
openProjectUrl: loadState('integration_openproject', 'openproject-url'),
244238
availableProjects: [],
@@ -267,10 +261,6 @@ export default {
267261
state: STATE.OK,
268262
}),
269263
computed: {
270-
openModal() {
271-
this.searchForProjects()
272-
return this.showModal
273-
},
274264
getSelectedProject() {
275265
return this.project.label
276266
},
@@ -334,6 +324,9 @@ export default {
334324
return dompurify.sanitize(message, { ADD_ATTR: ['target'] })
335325
},
336326
},
327+
beforeMount() {
328+
this.searchForProjects()
329+
},
337330
methods: {
338331
mappedProjects() {
339332
const mappedNodes = []
@@ -450,50 +443,66 @@ export default {
450443
}
451444
}
452445
const url = generateOcsUrl('/apps/integration_openproject/api/v1/projects')
446+
let projects = {}
453447
try {
448+
this.availableProjects = []
454449
const response = await axios.get(url, req)
455-
await this.processProjects(response.data.ocs.data)
450+
projects = response.data.ocs.data
456451
} catch (e) {
457-
console.error('Couldn\'t fetch openproject projects')
452+
console.error('Could not fetch OpenProject projects:', e.message)
458453
}
454+
455+
this.availableProjects = this.processProjects(projects)
456+
459457
if (this.isFetchingProjectsFromOpenProjectWithQuery === false) {
460458
this.initialAvailableProjects = this.availableProjects
461459
}
462460
if (this.isStateLoading) {
463461
this.state = STATE.OK
464462
}
465463
},
466-
async processProjects(projects) {
467-
this.availableProjects = []
468-
for (const index in projects) {
469-
const project = {}
470-
project.label = projects[index].name
471-
project.id = projects[index].id
472-
project.identifier = projects[index].identifier
473-
project.self = projects[index]._links.self
474-
project.parent = projects[index]._links.parent
475-
project.children = []
476-
this.availableProjects[index] = project
464+
processProjects(projects) {
465+
const flatProjectsStore = []
466+
for (const projectId in projects) {
467+
const opProject = projects[projectId]
468+
const project = {
469+
label: opProject.name,
470+
id: opProject.id,
471+
identifier: opProject.identifier,
472+
self: opProject._links.self,
473+
parent: opProject._links.parent,
474+
children: [],
475+
}
476+
flatProjectsStore[projectId] = project
477477
}
478-
this.buildNestedStructure()
479-
},
480-
buildNestedStructure() {
481-
const childId = []
482478
483-
for (const projectId in this.availableProjects) {
484-
const project = this.availableProjects[projectId]
485-
if (project.parent.href !== null) {
486-
const parentProjectId = project.parent.href.match(/\/(\d+)$/)[1]
487-
if (project.children.length <= 0) {
488-
this.availableProjects[parentProjectId].children.push(project)
489-
childId.push(project.id)
479+
return this.buildNestedProjects(projects, flatProjectsStore).filter(project => !!project)
480+
},
481+
buildNestedProjects(allProjects, flatProjectsStore) {
482+
const parentProjects = []
483+
for (const project of Object.values(allProjects)) {
484+
const parentHref = project._links.parent.href
485+
if (parentHref) {
486+
const parentMatch = parentHref.match(/\/(\d+)$/)
487+
if (parentMatch === null) {
488+
// if parent project is undisclosed, we consider it as a top-level project
489+
parentProjects.push(flatProjectsStore[project.id])
490+
continue
491+
}
492+
493+
const parentId = parentMatch[1]
494+
if (!(parentId in flatProjectsStore)) {
495+
// if parent project is not in the list of projects fetched, we consider it as a top-level project
496+
parentProjects.push(flatProjectsStore[project.id])
497+
continue
490498
}
499+
500+
flatProjectsStore[parentId].children.push(flatProjectsStore[project.id])
501+
} else {
502+
parentProjects.push(flatProjectsStore[project.id])
491503
}
492504
}
493-
for (let i = 0; i < childId.length; i++) {
494-
delete this.availableProjects[childId[i]]
495-
}
496-
this.availableProjects = this.availableProjects.filter(item => item !== undefined)
505+
return parentProjects
497506
},
498507
onSubjectChange(value) {
499508
if (this.error.error) {
@@ -603,7 +612,7 @@ export default {
603612
this.previousDescriptionTemplate = this.description.raw
604613
}
605614
} catch (e) {
606-
console.error('Form validation failed')
615+
console.error('Form validation failed:', e.message)
607616
}
608617
},
609618
setAllowedValues(allowedValuesList) {
@@ -637,7 +646,7 @@ export default {
637646
try {
638647
response = await axios.get(url)
639648
} catch (e) {
640-
console.error('Cannot fetch available assignees')
649+
console.error('Cannot fetch available assignees:', e.message)
641650
}
642651
const assignees = response.data.ocs.data
643652
for (const index in assignees) {

‎tests/jest/components/tab/SearchInput.spec.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -996,7 +996,7 @@ describe('SearchInput.vue', () => {
996996
})
997997

998998
describe('create work package button at the footer of the NcSelect', () => {
999-
wrapper = mountSearchInput()
999+
const wrapper = mountSearchInput()
10001000
it('should open create work package modal when clicked', async () => {
10011001
await wrapper.setData({
10021002
isSmartPicker: false,

‎tests/jest/views/CreateWorkpackageModal.spec.js‎

Lines changed: 67 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -82,16 +82,66 @@ describe('CreateWorkPackageModal.vue', () => {
8282
})
8383

8484
describe('workpackage creation form', () => {
85+
let axiosSpy
86+
afterEach(() => {
87+
axiosSpy.mockRestore()
88+
jest.clearAllMocks()
89+
})
90+
8591
it('should display available projects in the project dropdown', async () => {
86-
const axiosSpy = jest.spyOn(axios, 'get')
92+
axiosSpy = jest.spyOn(axios, 'get')
8793
.mockImplementationOnce(() => sendOCSResponse(availableProjectsResponse))
8894
wrapper = mountWrapper(true)
8995
expect(wrapper.find(createWorkPackageSelector).isVisible()).toBe(true)
9096
expect(axiosSpy).toHaveBeenCalledWith(projectsUrl, {})
9197
await wrapper.find(projectInputField).setValue(' ')
9298
expect(wrapper.find(projectSelectSelector)).toMatchSnapshot()
93-
axiosSpy.mockRestore()
94-
jest.clearAllMocks()
99+
})
100+
101+
it('should display projects even when parent project is unknown', async () => {
102+
const projectsListSelector = 'transition-stub > ul > li'
103+
const projectItemSelector = `${projectsListSelector}:nth-child(%s) > span`
104+
105+
axiosSpy = jest.spyOn(axios, 'get')
106+
.mockImplementationOnce(() => sendOCSResponse({
107+
4: {
108+
name: 'Child Project',
109+
id: 4,
110+
identifier: 'child-project',
111+
_links: {
112+
self: {
113+
href: '/api/v3/projects/4',
114+
title: 'Child Project',
115+
},
116+
parent: {
117+
href: 'urn:openproject-org:api:v3:undisclosed',
118+
title: 'Undisclosed - The parent is invisible because of lacking permissions.',
119+
},
120+
},
121+
},
122+
5: {
123+
name: 'Another Project',
124+
id: 5,
125+
identifier: 'another-project',
126+
_links: {
127+
self: {
128+
href: '/api/v3/projects/5',
129+
title: 'Another Project',
130+
},
131+
parent: {
132+
href: '/api/v3/projects/1',
133+
title: 'Parent Project',
134+
},
135+
},
136+
},
137+
}))
138+
wrapper = mountWrapper(true)
139+
expect(wrapper.find(createWorkPackageSelector).isVisible()).toBe(true)
140+
expect(axiosSpy).toHaveBeenCalledWith(projectsUrl, {})
141+
await wrapper.find(projectInputField).setValue(' ')
142+
expect(wrapper.find(projectsListSelector).exists()).toBe(true)
143+
expect(wrapper.find(util.format(projectItemSelector, 1)).text()).toBe('Another Project')
144+
expect(wrapper.find(util.format(projectItemSelector, 2)).text()).toBe('Child Project')
95145
})
96146

97147
describe('search projects with query', () => {
@@ -242,7 +292,7 @@ describe('CreateWorkPackageModal.vue', () => {
242292
.mockImplementationOnce(() => sendOCSResponse(workpackageFormValidationProjectSelected))
243293
const assigneeAxiosSpy = jest.spyOn(axios, 'get')
244294
.mockImplementationOnce(() => sendOCSResponse(availableProjectAssignees))
245-
wrapper = mountWrapper(true, {
295+
wrapper = mountWrapper({
246296
noDropAvailableProjectDropDown: false,
247297
})
248298
wrapper.vm.mappedProjects = jest.fn(() => {
@@ -377,7 +427,7 @@ describe('CreateWorkPackageModal.vue', () => {
377427
]
378428
const axiosSpy = jest.spyOn(axios, 'post')
379429
.mockImplementationOnce(() => sendOCSResponse(workpackageFormValidationTypeChanged))
380-
wrapper = mountWrapper(true, {
430+
wrapper = mountWrapper({
381431
allowedStatues: availableStatusBefore,
382432
allowedTypes,
383433
project: {
@@ -463,7 +513,7 @@ describe('CreateWorkPackageModal.vue', () => {
463513
.mockImplementationOnce(() => sendOCSResponse(availableProjectsResponse))
464514
const axiosSpy = jest.spyOn(axios, 'post')
465515
.mockImplementationOnce(() => sendOCSResponse(expectedErrorDetails.data, 422))
466-
wrapper = mountWrapper(true, {
516+
wrapper = mountWrapper({
467517
status: {
468518
label: 'New',
469519
},
@@ -512,7 +562,7 @@ describe('CreateWorkPackageModal.vue', () => {
512562
.mockImplementationOnce(() => sendOCSResponse(availableProjectsResponse))
513563
const axiosSpy = jest.spyOn(axios, 'post')
514564
.mockImplementationOnce(() => sendOCSResponse("{\"_type\":\"Error\",\"errorIdentifier\":\"urn:openproject-org:api:v3:errors:MultipleErrors\",\"message\":\"Multiple field constraints have been violated.\",\"_embedded\":{\"errors\":[{\"_type\":\"Error\",\"errorIdentifier\":\"urn:openproject-org:api:v3:errors:PropertyConstraintViolation\",\"message\":\"Subject can't be blank.\",\"_embedded\":{\"details\":{\"attribute\":\"subject\"}}},{\"_type\":\"Error\",\"errorIdentifier\":\"urn:openproject-org:api:v3:errors:PropertyConstraintViolation\",\"message\":\"Project can't be blank.\",\"_embedded\":{\"details\":{\"attribute\":\"project\"}}}]}}", 422))
515-
wrapper = mountWrapper(true, {
565+
wrapper = mountWrapper({
516566
status: {
517567
label: 'New',
518568
},
@@ -536,7 +586,7 @@ describe('CreateWorkPackageModal.vue', () => {
536586
const assigneeAxiosSpy = jest.spyOn(axios, 'get')
537587
.mockImplementationOnce(() => sendOCSResponse(availableProjectAssignees))
538588

539-
wrapper = mountWrapper(true, {
589+
wrapper = mountWrapper({
540590
project: {
541591
self: {
542592
href: '/api/v3/projects/4',
@@ -591,7 +641,7 @@ describe('CreateWorkPackageModal.vue', () => {
591641
const assigneeAxiosSpy = jest.spyOn(axios, 'get')
592642
.mockImplementationOnce(() => sendOCSResponse(availableProjectAssignees))
593643

594-
wrapper = mountWrapper(true, {
644+
wrapper = mountWrapper({
595645
project: {
596646
self: {
597647
href: '/api/v3/projects/4',
@@ -638,7 +688,7 @@ describe('CreateWorkPackageModal.vue', () => {
638688
const assigneeAxiosSpy = jest.spyOn(axios, 'get')
639689
.mockImplementationOnce(() => sendOCSResponse(availableProjectAssignees))
640690

641-
wrapper = mountWrapper(true, {
691+
wrapper = mountWrapper({
642692
project: {
643693
self: {
644694
href: '/api/v3/projects/4',
@@ -671,7 +721,7 @@ describe('CreateWorkPackageModal.vue', () => {
671721
const assigneeAxiosSpy = jest.spyOn(axios, 'get')
672722
.mockImplementationOnce(() => sendOCSResponse(availableProjectAssignees))
673723

674-
wrapper = mountWrapper(true, {
724+
wrapper = mountWrapper({
675725
project: {
676726
self: {
677727
href: '/api/v3/projects/4',
@@ -724,7 +774,7 @@ describe('CreateWorkPackageModal.vue', () => {
724774
},
725775
])('should show $expectedMessage when project is set and there is no $fieldName found in search query', async ({ inputSelector, resultSelector, expectedMessage }) => {
726776

727-
wrapper = mountWrapper(true, {
777+
wrapper = mountWrapper({
728778
project: {
729779
self: {
730780
href: '/api/v3/projects/4',
@@ -777,7 +827,7 @@ describe('CreateWorkPackageModal.vue', () => {
777827
.mockImplementationOnce(() => sendOCSResponse(availableProjectsResponse))
778828
const axiosSpy = jest.spyOn(axios, 'post')
779829
.mockImplementationOnce(() => sendOCSResponse(workpackageCreatedResponse, 201))
780-
wrapper = mountWrapper(true, {
830+
wrapper = mountWrapper({
781831
project: {
782832
self: {
783833
href: '/api/v3/projects/2',
@@ -855,7 +905,7 @@ describe('CreateWorkPackageModal.vue', () => {
855905
const axiosSpy = jest.spyOn(axios, 'post')
856906
.mockImplementationOnce(() => sendOCSResponse(requiredTypeResponse))
857907

858-
wrapper = mountWrapper(true, {
908+
wrapper = mountWrapper({
859909
project: {
860910
self: {
861911
href: '/api/v3/projects/4',
@@ -903,7 +953,7 @@ describe('CreateWorkPackageModal.vue', () => {
903953
jest.spyOn(axios, 'get')
904954
.mockImplementationOnce(() => sendOCSResponse(availableProjectsResponse))
905955

906-
wrapper = mountWrapper(true, {
956+
wrapper = mountWrapper({
907957
project: {
908958
self: {
909959
href: '/api/v3/projects/4',
@@ -939,7 +989,7 @@ describe('CreateWorkPackageModal.vue', () => {
939989
})
940990

941991
it('should be able to remove the selected project', async () => {
942-
wrapper = mountWrapper(true, {
992+
wrapper = mountWrapper({
943993
project: {
944994
self: {
945995
href: '/api/v3/projects/2',
@@ -966,7 +1016,7 @@ function sendOCSResponse(data, status = 200) {
9661016
})
9671017
}
9681018

969-
function mountWrapper(showModal, data) {
1019+
function mountWrapper(data) {
9701020
return mount(CreateWorkPackageModal, {
9711021
localVue,
9721022
mocks: {
@@ -978,8 +1028,5 @@ function mountWrapper(showModal, data) {
9781028
stubs: {
9791029
NcModal: true,
9801030
},
981-
propsData: {
982-
showModal,
983-
},
9841031
})
9851032
}

0 commit comments

Comments
 (0)