[PM-27933] Skip assign tasks view if no critical applications are selected

[Banrion]

🎟️ Tracking

PM-27933

📔 Objective

This pull requests adds the logic to skip the assign tasks view if the user has selected critical applications that do not have at risk passwords to assign tasks to.

A simple dialog is also added to the review page to confirm when a user has not selected any critical applications.

Changes

• Skip assign tasks view when reviewing applications if there are no ciphers to assign tasks to
• Adds simple dialog to confirm when no critical applications have been selected in the review process

📸 Screenshots

This recording show skipping if the at risk cipher already had a password assigned.

Screen.Recording.2025-11-12.at.1.50.10.PM.mov

This screenshot shows that the no-tasks.com applications was saved as a critical application.

The following recording shows the assign tasks screen when the selected applications has tasks to assign.

Screen.Recording.2025-11-12.at.1.54.02.PM.mov

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[claude]

Claude finished @Banrion's task —— View job

---

Code Review Complete

Summary

This PR successfully implements logic to skip the assign tasks view when no at-risk passwords need assignment and adds a confirmation dialog for empty selections. The refactoring to use computed signals is well-aligned with Angular best practices (ADR-0027).

Critical Issues (❌)

Finding 1: Missing protection against rapid clicks in handleMarkAsCritical (new-applications-dialog.component.ts:217)

• The markingAsCritical signal guard was removed when this method became async
• Button remains enabled between confirmation dialog and view transition
• Impact: Rapid clicks could trigger multiple view transitions or dialogs
• See inline comment for fix

Finding 2: Redundant takeUntilDestroyed on line 259

• The outer pipe already handles cleanup with takeUntilDestroyed and take(1)
• Inner takeUntilDestroyed on line 261 serves no purpose
• Impact: Unnecessary subscription overhead
• See inline comment for fix

Finding 3: Unnecessarily complex observable chain (lines 261-268)

• Since newUnassignedAtRiskCipherIds() is a computed signal, no need to convert via toSignal().pipe()
• Can directly access the computed value
• Impact: Reduced readability and unnecessary complexity
• See inline comment for fix

Minor Issues (⚠️)

Finding 4: markingAsCritical signal is no longer functional

• Signal is bound to button's [loading] and [disabled] attributes (lines 57-58 in HTML)
• But never set to true/false anymore since method became async
• Button states are effectively never updated by this signal

Finding 5: Inconsistent localization formatting in messages.json

• New entries have spaces before colons: "message" : instead of "message":
• Rest of file uses no spaces
• See inline comment for fix

Code Quality (🎨)

Finding 6: Constructor parameter reordering appears arbitrary

• Parameters reordered: dialogRef, dialogService, dataService moved
• No functional impact but reduces git blame accuracy
• Consider maintaining original order when adding new dependencies

Finding 7: Inconsistent documentation style

• Some methods have JSDoc (/** ... */), others have single-line comments (//)
• Examples: handleMarkAsCritical (line comment) vs open method (JSDoc)

Good Practices Observed

• Proper use of const object pattern over enums (ADR-0025)
• Effective use of computed signals for derived state (ADR-0027)
• Proper cleanup with takeUntilDestroyed
• Clear separation of concerns between review and assign flows

Action Items

1. ❌ Add signal guard to prevent rapid clicks (Finding 1)
2. ❌ Remove redundant takeUntilDestroyed (Finding 2)
3. ❌ Simplify observable chain (Finding 3)
4. ⚠️ Fix or remove non-functional markingAsCritical signal (Finding 4)
5. ⚠️ Fix localization formatting inconsistency (Finding 5)
6. 🎨 Consider: Standardize documentation style (Finding 7)

[claude]

❌ The markingAsCritical signal guard was removed when this method became async. This could allow rapid clicks to bypass the protection. Consider adding:

if (this.markingAsCritical()) {
  return;
}
this.markingAsCritical.set(true);

And then set it to false after the dialog interaction completes.

[github-actions]

Checkmarx One – Scan Summary & Details – c421a959-2855-4d48-b889-96960e7ae867

Great job! No new security vulnerabilities introduced in this pull request

[codecov]

Codecov Report

❌ Patch coverage is 0% with 44 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.70%. Comparing base (089caf5) to head (054f43e).
⚠️ Report is 24 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...review-dialog/new-applications-dialog.component.ts	0.00%	36 Missing ⚠️
...activity-cards/password-change-metric.component.ts	0.00%	8 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #17351   +/-   ##
=======================================
  Coverage   40.69%   40.70%           
=======================================
  Files        3539     3541    +2     
  Lines      101351   101421   +70     
  Branches    15181    15189    +8     
=======================================
+ Hits        41248    41283   +35     
- Misses      58360    58396   +36     
+ Partials     1743     1742    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.