Release: promote master into stable - #8060
Merged
Merged
Conversation
Bumps [@angular/common](https://github.com/angular/angular/tree/HEAD/packages/common) from 20.3.16 to 20.3.27. - [Release notes](https://github.com/angular/angular/releases) - [Changelog](https://github.com/angular/angular/blob/main/CHANGELOG.md) - [Commits](https://github.com/angular/angular/commits/v20.3.27/packages/common) --- updated-dependencies: - dependency-name: "@angular/common" dependency-version: 20.3.27 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…ent (#7995) Bumps [@angular/core](https://github.com/angular/angular/tree/HEAD/packages/core) from 20.3.17 to 20.3.27. - [Release notes](https://github.com/angular/angular/releases) - [Changelog](https://github.com/angular/angular/blob/main/CHANGELOG.md) - [Commits](https://github.com/angular/angular/commits/v20.3.27/packages/core) --- updated-dependencies: - dependency-name: "@angular/core" dependency-version: 20.3.27 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [immutable](https://github.com/immutable-js/immutable-js) from 5.1.5 to 5.1.9. - [Release notes](https://github.com/immutable-js/immutable-js/releases) - [Changelog](https://github.com/immutable-js/immutable-js/blob/main/CHANGELOG.md) - [Commits](immutable-js/immutable-js@v5.1.5...v5.1.9) --- updated-dependencies: - dependency-name: immutable dependency-version: 5.1.9 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [sigstore](https://github.com/sigstore/sigstore-js) from 4.1.0 to 4.1.1. - [Release notes](https://github.com/sigstore/sigstore-js/releases) - [Commits](https://github.com/sigstore/sigstore-js/compare/sigstore@4.1.0...sigstore@4.1.1) --- updated-dependencies: - dependency-name: sigstore dependency-version: 4.1.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [form-data](https://github.com/form-data/form-data) from 4.0.5 to 4.0.6. - [Changelog](https://github.com/form-data/form-data/blob/master/CHANGELOG.md) - [Commits](form-data/form-data@v4.0.5...v4.0.6) --- updated-dependencies: - dependency-name: form-data dependency-version: 4.0.6 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
#7954) Bumps [launch-editor](https://github.com/vitejs/launch-editor) from 2.13.0 to 2.14.1. - [Commits](vitejs/launch-editor@v2.13.0...v2.14.1) --- updated-dependencies: - dependency-name: launch-editor dependency-version: 2.14.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…7996) Bumps [ip-address](https://github.com/beaugunderson/ip-address) from 10.2.0 to 10.4.0. - [Release notes](https://github.com/beaugunderson/ip-address/releases) - [Commits](beaugunderson/ip-address@v10.2.0...v10.4.0) --- updated-dependencies: - dependency-name: ip-address dependency-version: 10.4.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [tar](https://github.com/isaacs/node-tar) from 7.5.11 to 7.5.22. - [Release notes](https://github.com/isaacs/node-tar/releases) - [Changelog](https://github.com/isaacs/node-tar/blob/main/CHANGELOG.md) - [Commits](isaacs/node-tar@v7.5.11...v7.5.22) --- updated-dependencies: - dependency-name: tar dependency-version: 7.5.22 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [tmp](https://github.com/raszi/node-tmp) from 0.2.5 to 0.2.7. - [Changelog](https://github.com/raszi/node-tmp/blob/master/CHANGELOG.md) - [Commits](raszi/node-tmp@v0.2.5...v0.2.7) --- updated-dependencies: - dependency-name: tmp dependency-version: 0.2.7 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
--- updated-dependencies: - dependency-name: Sentry dependency-version: 6.10.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…#8029) Bumps [browserslist](https://github.com/browserslist/browserslist) from 4.28.1 to 4.28.8. - [Release notes](https://github.com/browserslist/browserslist/releases) - [Changelog](https://github.com/browserslist/browserslist/blob/main/CHANGELOG.md) - [Commits](browserslist/browserslist@4.28.1...4.28.8) --- updated-dependencies: - dependency-name: browserslist dependency-version: 4.28.8 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
… its error (#8036) POST /api/template-files/image persisted UploadedData and FieldValue before the step that can fail (thumbnail generation), returned an untranslated resource key on failure, and the client let the user re-submit the same file on every further click. One transient failure became a raw-key toast and N duplicate picture cards after N retries. The endpoint also stored Extension without the leading dot, so every web-uploaded picture was unreachable through get-image/{fileName}.{ext} and invisible in the compliance image gallery. Backend (EFormFilesController.AddNewImage) - Thumbnails are derived to temp names before any row is written; the UploadedData/FieldValue creation and the storage puts run in one transaction. The SDK context uses EnableRetryOnFailure, so the unit runs through Database.CreateExecutionStrategy().ExecuteAsync with the entities built inside the delegate and the change tracker cleared on entry, so a transient retry cannot re-insert a failed attempt. - Idempotency guard: a non-removed FieldValue on (CaseId, FieldId) whose non-removed UploadedData has the same checksum returns success without creating anything; delete-then-reupload still creates a new row. - Extension stored with the leading dot (".png"); FileName keeps the {id}_{md5}.{ext} shape. - Temp files (GUID stem) removed in finally on every path; the exception is logged with its stack trace and structured case/field ids. - SharedResource.resx / .da.resx: ErrorWhileUpdateImage and ImageNotFound (neutral English + real Danish); no placeholders in other locales. Frontend (element-picture) - Host buttons and the dialog's Save are locked while the upload is in flight; released on success:false and on error so the user can retry. - "Add new image" label translated; AddPictureDialog renders Cancel before Save. Verified against the local dev stack: dotted extension, idempotent re-POST, thumbnails served under the dotted names, delete-then-reupload creates a new row, corrupt file yields the translated error with no rows and no temp leftovers. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018qJL2WhHwhZ5CGZehZF2ro
fix(eform-files): make picture upload atomic and idempotent; localize its error (#8036)
`eform-client/TESTING.md` already documented `npm run test:ci` as what CI runs. The workflows were the outlier, still calling `test:unit`. This aligns them. Behaviourally this is close to neutral, and the doc now says so rather than implying a benefit: - `--ci` is redundant on GitHub Actions. Jest auto-detects CI via `ci-info`, which lists `GITHUB_ACTIONS`; `jest --showConfig` resolves `ci: true` on a runner without the flag. It is kept so the script stays correct anywhere detection does not fire. - `--maxWorkers=2` is dropped from `test:ci`. It was an unexamined number, and it buys nothing measurable: benchmarked on a 4-CPU cgroup with a cold cache, capped 239.7s vs default 240.1s over two runs each, with the two modes trading places. Peak RSS ~2.65 GB against 16 GB either way. Removing it lets Jest scale to whatever runner the repo gets rather than pinning a constant nobody re-measured. TESTING.md's CI section now names the job and both workflow files, states the worker behaviour accurately, and records that these jobs are not required status checks — so a failing Jest run does not by itself block a merge. Not fixed here, but worth knowing: the rest of TESTING.md is stale in ways a consistency touch-up should not absorb. Its entire "Testing Patterns" section teaches Jasmine (`jasmine.createSpyObj`), which no spec in the repo uses and `src/setup-jest.ts` explicitly says was removed; "Common Issues" and "Debugging Tests" reference `karma.conf.js` and `src/test.ts`, neither of which exists. That is a doc rewrite, not this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018qJL2WhHwhZ5CGZehZF2ro
ci: point the Jest job at test:ci, and correct TESTING.md
…iFI, huHU (#8058) enUS is 'Generate {{value}} report'. Three locales lost the token, so ngx-translate finds no matching parameter and the report name never renders. Two are the translated-placeholder-name bug: fiFI had '{{arvo}}' and huHU '{{érték}}' — an earlier translateTsFiles.py run sent the whole string to translation including the {{...}} tokens, so the placeholder NAMES were translated and ngx-translate renders the braces literally. Same defect as microting/eform-backendconfiguration-plugin#1181, which fixed 28 such entries in that plugin's i18n. Danish is the worse shape: {{value}} had been dropped ENTIRELY, translated away into plain prose ('Generer rapport'). There was nothing to substitute, so the token had to be reinserted. Placed as a compound modifier, 'Generer {{value}}-rapport', matching noNO for the same key — Danish and Norwegian take the same compound-noun hyphen here. A dropped token has no textual signature; only a token-multiset comparison against enUS finds it. Hungarian keeps its own word order — the token legitimately leads there, so only the name was substituted. All 26 shipped locales now carry the token. The only files without it are translates.ts and template.ts, the latter being an unimported scaffold whose values are all empty strings. Found by running the i18n guard added in eform-backendconfiguration-plugin across every i18n directory in the workspace. Note that guard hardcodes 'enUS.ts' as its reference and has no JSON support, so 24 of 29 directories abort before checking anything — it needs widening before it can be reused outside the plugin it was written for. Claude-Session: https://claude.ai/code/session_018qJL2WhHwhZ5CGZehZF2ro Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Only the first user (lowest id), who also holds the delete claim, can delete a device user or a site. DeviceUsersService.Delete and SitesService.Delete share one check (FirstUserHelper) and refuse anyone else before any SDK call. The /device-users and Sites delete items show only for that user. RefreshToken now returns IsFirstUser like login does. Edit and New OTP on /device-users follow the update claim. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Wuv2gtqY9BLMLWBZiBM2Y
|
|
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate upload and error-handling findings, plus a critical test setup finding, remain.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Promotes master into stable with first-user authorization, reliable picture uploads, localization, CI, and dependency updates.
Changes:
- Restricts device-user and site deletion to the first user.
- Adds transactional, idempotent picture-upload handling.
- Updates translations, tests, CI configuration, documentation, and dependencies.
File summaries
| File | Summary |
|---|---|
eFormAPI/eFormAPI.Web/Services/SitesService.cs |
Enforces first-user site deletion. |
eFormAPI/eFormAPI.Web/Services/DeviceUsersService.cs |
Enforces first-user device-user deletion. |
eFormAPI/eFormAPI.Web/Services/AuthService.cs |
Preserves first-user status during refresh. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.uk.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.sv.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.sl.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.sk.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.ro.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.resx |
Adds English fallback messages. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.pt.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.pl.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.no.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.nl.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.lv.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.lt.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.it.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.is.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.hu.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.hr.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.fr.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.fi.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.et.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.es.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.el.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.de.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.da.resx |
Adds localized deletion and image messages. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.cs.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Resources/SharedResource.bg.resx |
Adds localized deletion text. |
eFormAPI/eFormAPI.Web/Infrastructure/Helpers/FirstUserHelper.cs |
Implements first-user detection. |
eFormAPI/eFormAPI.Web/eFormAPI.Web.csproj |
Updates Sentry. |
eFormAPI/eFormAPI.Web/Controllers/Eforms/EFormFilesController.cs |
Adds transactional image-upload handling. |
eFormAPI/eFormAPI.Web.Integration.Tests/Services/SitesServiceTests.cs |
Updates site service tests. |
eFormAPI/eFormAPI.Web.Integration.Tests/Services/SitesServiceDeleteTests.cs |
Tests site deletion authorization. |
eFormAPI/eFormAPI.Web.Integration.Tests/Services/FirstUserOnlyDeleteTestsBase.cs |
Provides shared deletion tests. |
eFormAPI/eFormAPI.Web.Integration.Tests/Services/DeviceUsersServiceTests.cs |
Updates device-user service tests. |
eFormAPI/eFormAPI.Web.Integration.Tests/Services/DeviceUsersServiceDeleteTests.cs |
Tests device-user deletion authorization. |
eFormAPI/eFormAPI.Web.Integration.Tests/Services/AuthServiceTests.cs |
Tests refreshed first-user state. |
eform-client/yarn.lock |
Locks dependency updates. |
eform-client/TESTING.md |
Documents Jest CI behavior. |
eform-client/src/test-helpers.ts |
Adds Angular test mocks. |
eform-client/src/assets/i18n/huHU.ts |
Restores the report placeholder. |
eform-client/src/assets/i18n/fiFI.ts |
Restores the report placeholder. |
eform-client/src/assets/i18n/da.ts |
Restores the report placeholder. |
eform-client/src/app/modules/device-users/components/device-users-page/device-users-page.component.ts |
Applies first-user permissions. |
eform-client/src/app/modules/device-users/components/device-users-page/device-users-page.component.spec.ts |
Tests device-user permissions. |
eform-client/src/app/modules/device-users/components/device-users-page/device-users-page.component.html |
Updates device-user actions. |
eform-client/src/app/modules/advanced/components/sites/sites/sites.component.ts |
Applies first-user site permissions. |
eform-client/src/app/modules/advanced/components/sites/sites/sites.component.spec.ts |
Tests site permissions. |
eform-client/src/app/modules/advanced/components/sites/sites/sites.component.html |
Updates site actions. |
eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-picture/element-picture.component.ts |
Adds upload locking and retry handling. |
eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-picture/element-picture.component.html |
Translates the image-upload label. |
eform-client/package.json |
Updates test scripts and Angular dependencies. |
.github/workflows/dotnet-core-pr.yml |
Uses the CI Jest command. |
.github/workflows/dotnet-core-master.yml |
Uses the CI Jest command. |
Review details
Suppressed comments (2)
eFormAPI/eFormAPI.Web/Controllers/Eforms/EFormFilesController.cs:355
- Because this delegate includes
CommitAsyncbut is run throughExecuteAsyncwithout a success-verification callback, a transient exception after the database has actually committed can cause the execution strategy to invoke it again.ChangeTracker.Clear()then creates a new row pair with a new id, so one upload can still produce duplicates (and extra storage objects). Use a transaction-execution API with verification keyed by case, field, and checksum, or otherwise make the database operation idempotent across retries.
await strategy.ExecuteAsync(async () =>
eFormAPI/eFormAPI.Web/Controllers/Eforms/EFormFilesController.cs:320
- The new upload path adds transaction, thumbnail, idempotency, and cleanup branches, but
EFormFilesControllerTestsonly contains an initialization smoke test and does not exerciseAddNewImage. Please add automated coverage for duplicate no-op behavior, thumbnail failure/rollback and cleanup, and storage failure so these failure paths are protected.
// Idempotency guard: the same file already attached to this
// (case, field) is a no-op success. Both "not removed" conditions
// are load-bearing — DeleteImage soft-deletes only the
// UploadedData, so a delete-then-reupload must create a new row.
var alreadyAttached = await sdkDbContext.FieldValues
.Where(x => x.WorkflowState != Constants.WorkflowStates.Removed)
.Where(x => x.CaseId == caseDb.Id)
.Where(x => x.FieldId == field.Id)
.Where(x => x.UploadedDataId != null)
.AnyAsync(x => x.UploadedData.WorkflowState != Constants.WorkflowStates.Removed
&& x.UploadedData.Checksum == hash);
- Files reviewed: 53/54 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| template: ` | ||
| @if (hasActionsColumn) { | ||
| @for (row of data ?? []; track $index) { | ||
| <ng-container *ngTemplateOutlet="cellTemplate?.actions; context: {$implicit: row, index: $index}"></ng-container> |
Comment on lines
+314
to
+318
| var alreadyAttached = await sdkDbContext.FieldValues | ||
| .Where(x => x.WorkflowState != Constants.WorkflowStates.Removed) | ||
| .Where(x => x.CaseId == caseDb.Id) | ||
| .Where(x => x.FieldId == field.Id) | ||
| .Where(x => x.UploadedDataId != null) |
Comment on lines
275
to
284
| public async Task<OperationResult> Delete(int id) | ||
| { | ||
| // Only the first user may delete a device user; everyone else is refused before the SDK. | ||
| if (!await userService.IsFirstUserAsync()) | ||
| { | ||
| return new OperationResult(false, | ||
| localizationService.GetString("OnlyTheFirstUserCanDeleteWorkers")); | ||
| } | ||
|
|
||
| try |
Comment on lines
243
to
252
| public async Task<OperationResult> Delete(int id) | ||
| { | ||
| // Only the first user may delete a site (a device user); everyone else is refused before the SDK. | ||
| if (!await userService.IsFirstUserAsync()) | ||
| { | ||
| return new OperationResult(false, | ||
| localizationService.GetString("OnlyTheFirstUserCanDeleteWorkers")); | ||
| } | ||
|
|
||
| try |
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
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.
Promotes
masterintostable(17 commits):Merge with a merge commit so
masterandstablestay in sync.🤖 Generated with Claude Code
https://claude.ai/code/session_014Wuv2gtqY9BLMLWBZiBM2Y