Repository navigation
fix(review): #1269 review findings — a missing CMDB sheet archives instead of deleting - #1305
Open
WilcoLouwerse wants to merge 5 commits into
Open
WilcoLouwerse wants to merge 5 commits into
WilcoLouwerse wants to merge 5 commits into
Conversation
…instead of deleting The reader refused a workbook only when none of the profile's source sheets was present, and the reconciliation treated the workbook as authoritative for a deletion whenever the archive sheet was readable. A workbook that had "Onbeh Applicaties CMDB" but not "Beheerde Applicaties CMDB" (a renamed sheet, a partial export) therefore soft-deleted every application an earlier import took from the missing sheet, on a plain re-import with the default missingRecords=archive, and the report gave no reason. The reader now returns the expected source sheets the workbook lacks (absentSheets). An import with such a sheet is not authoritative for a deletion, like one with an unreadable key: what is on no sheet is archived, never deleted, and the report warns that the sheet was not found. The run caches for unreadable keys and absent sheets are cleared with the others. Fixes #1304 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… lists Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…uuid technicalOwner is the same $ref contactPerson relation as businessOwner, whose column #1292 gave the contactName widget; the technical owner column still printed the key. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…chive sheet
The info paragraph already switched on the archive sheet, but the choice
"Archive or delete them", its help ("archived or moved to the trash") and
the re-import line ("archived or deleted") did not. Without an archive
sheet the import only archives, so these now read "Archive them" and
"archived" for such a profile.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… why they have no screen 118 capabilities named their board as a string and 44 as a list, so a reader iterating board walked a string character by character. Every board is a list now, as in specScreens. The 41 decided-no rows had no screen at all; they get board null with the reason that nothing was built. The file still validates against the parity-verify capabilities schema. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Quality Report — ConductionNL/stackiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-vue-demi | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ❌ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 133/133 | |||
| npm | ✅ | ✅ 807/807 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ✅ 1428/1428 · 20 skipped | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it locally with npx playwright test, or from the Actions tab on a branch with no open pull request into development. |
||||
| Hydra gates | ✅ |
Quality workflow — 2026-10-16 12:00 UTC
Download the full PDF report from the workflow artifacts.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the review findings on the release PR #1269 (development → beta) that can be fixed in code. The release PR picks these up once this merges into
development.absentSheets). Such an import is not authoritative for a deletion, like one with an unreadable key: what is on no sheet is archived, never deleted, and the report warns that the sheet was not found. The REQ-CMDB-015 requirement and the feature docs describe the case.contactNamewidget, like the business owner column.screen.boardis always a list, and the 41 decided-no rows haveboard: nullwith a reason. The file still validates against the parity-verify schema.Left open
providernarrowing from "View all":FacetControllerhas no filter on arbitrary list keys, so forwarding the key from the frontend alone would not change the counts. That needs a backend filter, or hiding the sidebar while the list is pre-filtered. Cosmetic; the review marked it fine to leave for a later change.tests/e2e/spec-coverage/cmdb-import.spec.ts,cmdb-import-mapping.spec.ts) still have to be run on a rig with the head of development before the release. Task 7's checkbox stays unticked until then.Test plan
phpunit -c phpunit-unit.xml tests/Unit/Service/Cmdb tests/Unit/Service/CmdbExportImportServiceTest.phpwithOPENREGISTER_DIRset for PhpSpreadsheet: 268 tests green, including the newtestAMissingSourceSheetArchivesInsteadOfDeletingandtestAWorkbookWithSomeSourceSheetsNamesTheAbsentOnescomposer phpcs, phpmd (both rule sets, with the baseline),composer phpstan: cleanjest src/views/settings/sections/CmdbImport.spec.js src/utils/cmdbImport: 108 tests greennpm run check:manifest,npm run test:l10n,npm run check:l10n-js: cleanmodule.findPackageJSON, which@nextcloud/eslint-configneeds); CI runs it🤖 Generated with Claude Code