Frontend/2 year encumbrance privilege block#1011
Conversation
WalkthroughAdds a two-year wait-period check preventing purchases for two years after the most recent encumbrance was lifted; adds locale strings (EN/ES); updates License and Licensee models, tests, mocks, dashboard VM and view; and exports serverDatetimeFormat for test date formatting. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as LicenseeDashboard.vue
participant VM as LicenseeDashboard.ts
participant Licensee
participant License
User->>UI: Click "Obtain privileges"
UI->>VM: check canPurchasePrivileges()
VM->>Licensee: canPurchasePrivileges()
Licensee->>License: isLatestLiftedEncumbranceWithinWaitPeriod() for each privilege
License-->>Licensee: true/false
alt any true
Licensee-->>VM: hasEncumbranceLiftedWithinWaitPeriod = true
VM-->>UI: block purchase, show message
UI-->>User: display purchaseUnavailableEncumbranceLiftedWithinWaitPeriod
else all false
Licensee-->>VM: allowed
VM-->>UI: proceed with purchase flow
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
webroot/src/network/mocks/mock.data.ts (1)
1283-1288: Second OH adverse action: boundary set to exactly two years — matches acceptance criteriaSetting effectiveLiftDate to moment().subtract(2, 'years') is perfect for validating the exact-boundary case. Given the model uses endDate.isAfter(twoYearsAgo), equality won’t be considered “within” the wait period, which aligns with requirements.
Ensure tests cover this equality condition explicitly.
webroot/src/pages/LicenseeDashboard/LicenseeDashboard.vue (1)
127-129: New reason bullet renders — confirm translation key path across locales and consider conditionally hiding when encumbered
- Ensure the Spanish locale defines licensing.purchaseUnavailableEncumbranceLiftedWithinWaitPeriod (AI summary indicates it was added under common, which will 404).
- Optional: To avoid mixed messaging when a current encumbrance exists, consider only showing this bullet when not isEncumbered, since the “lifted within two years” message refers to previously ended encumbrances.
- <li v-if="hasEncumbranceLiftedWithinWaitPeriod"> + <li v-if="hasEncumbranceLiftedWithinWaitPeriod && !isEncumbered"> {{ $t('licensing.purchaseUnavailableEncumbranceLiftedWithinWaitPeriod') }} </li>webroot/src/models/License/License.model.ts (1)
198-222: LGTM! Well-implemented wait period logic with minor optimization opportunity.The method correctly implements the two-year wait period requirement by:
- Filtering adverse actions with creation dates
- Sorting by creation date to find the most recent
- Checking if the end date of the latest encumbrance is within two years
The logic is sound and handles edge cases properly (no adverse actions, no end date).
Consider extracting the wait period duration as a constant for better maintainability:
+ private static readonly WAIT_PERIOD_YEARS = 2; + public isLatestEncumbranceWithinWaitPeriod(): boolean { let withinWaitPeriod = false; if (this.adverseActions?.length) { const latestEncumbrance: AdverseAction = this.adverseActions .filter((adverseAction: AdverseAction) => adverseAction.creationDate) .sort((a: AdverseAction, b: AdverseAction) => { const dateA = moment(a.creationDate, serverDatetimeFormat); const dateB = moment(b.creationDate, serverDatetimeFormat); return dateB.valueOf() - dateA.valueOf(); // Most recent creationDate first })[0]; // Check if we found a latest adverse action and it has an end date if (latestEncumbrance?.endDate) { // Check if the end date is within the last 2 years (within wait period) const endDate = moment(latestEncumbrance.endDate, serverDateFormat); - const twoYearsAgo = moment().subtract(2, 'years'); + const waitPeriodAgo = moment().subtract(License.WAIT_PERIOD_YEARS, 'years'); - withinWaitPeriod = endDate.isAfter(twoYearsAgo); + withinWaitPeriod = endDate.isAfter(waitPeriodAgo); } } return withinWaitPeriod; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
webroot/src/locales/en.json(1 hunks)webroot/src/locales/es.json(1 hunks)webroot/src/models/License/License.model.spec.ts(7 hunks)webroot/src/models/License/License.model.ts(2 hunks)webroot/src/models/Licensee/Licensee.model.spec.ts(5 hunks)webroot/src/models/Licensee/Licensee.model.ts(1 hunks)webroot/src/network/mocks/mock.data.ts(3 hunks)webroot/src/pages/LicenseeDashboard/LicenseeDashboard.ts(1 hunks)webroot/src/pages/LicenseeDashboard/LicenseeDashboard.vue(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-30T13:48:25.375Z
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#824
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py:116-201
Timestamp: 2025-05-30T13:48:25.375Z
Learning: In encumbrance handling code, prefer to keep privilege and license encumbrance lifting implementations decoupled rather than extracting shared logic, as requirements between these implementations are likely to change in the future.
Applied to files:
webroot/src/models/Licensee/Licensee.model.tswebroot/src/models/Licensee/Licensee.model.spec.ts
🧬 Code Graph Analysis (5)
webroot/src/models/Licensee/Licensee.model.ts (1)
webroot/src/models/License/License.model.ts (1)
License(69-223)
webroot/src/models/License/License.model.ts (2)
webroot/src/models/AdverseAction/AdverseAction.model.ts (1)
AdverseAction(33-98)webroot/src/app.config.ts (2)
serverDatetimeFormat(101-101)serverDateFormat(100-100)
webroot/src/models/Licensee/Licensee.model.spec.ts (1)
webroot/src/models/Licensee/Licensee.model.ts (1)
Licensee(63-291)
webroot/src/models/License/License.model.spec.ts (2)
webroot/src/components/SelectedLicenseInfo/SelectedLicenseInfo.ts (1)
license(18-20)webroot/src/app.config.ts (2)
serverDatetimeFormat(101-101)serverDateFormat(100-100)
webroot/src/network/mocks/mock.data.ts (1)
webroot/src/app.config.ts (2)
serverDateFormat(100-100)serverDatetimeFormat(101-101)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CheckWebroot
🔇 Additional comments (19)
webroot/src/locales/en.json (1)
786-786: Translation key path matches between en and es — no action neededSearch results show the key exists in both locales under the same sibling context, so the earlier concern about Spanish being under a different path is not supported by the repository state.
- webroot/src/locales/en.json — line 786: purchaseUnavailableEncumbranceLiftedWithinWaitPeriod (English string)
- webroot/src/locales/es.json — line 770: purchaseUnavailableEncumbranceLiftedWithinWaitPeriod (Spanish string)
webroot/src/network/mocks/mock.data.ts (2)
1058-1068: OH privilege date anchors updated — confirm scenario alignment with boundary testThese date shifts (dateOfUpdate/Issuance/Renewal/Expiration) look consistent and should keep the privilege active while exercising the “exactly two years ago” boundary in adverse actions elsewhere.
Double-check that this provider’s latest encumbrance lift is set to exactly two years ago (see adverseActions below) to validate the “not blocked” scenario per test instructions.
1267-1272: AdverseAction serializer maps effectiveLiftDate → endDate — confirmedSerializer mapping is present and covered by tests; mock dates/formats look correct, so no change required.
- webroot/src/models/AdverseAction/AdverseAction.model.ts (lines ~113-114): startDate: json.effectiveStartDate, endDate: json.effectiveLiftDate
- webroot/src/models/AdverseAction/AdverseAction.model.spec.ts (line ~162): expect(adverseAction.endDate).to.equal(data.effectiveLiftDate)
- webroot/src/network/licenseApi/data.api.ts (lines ~299, ~373): effectiveLiftDate: endDate (model → API)
- webroot/src/network/mocks/mock.data.ts (lines ~1267–1272): mock values — creationDate uses serverDatetimeFormat; effectiveStartDate/effectiveLiftDate use serverDateFormat
webroot/src/models/Licensee/Licensee.model.ts (1)
286-290: Purchase gating expanded — good, but depends on accurate “last lift” semanticsAdding !this.hasEncumbranceLiftedWithinWaitPeriod() to canPurchasePrivileges is correct. Once you include license-level checks (above) and confirm the License model computes the latest lift by endDate, this will fully implement the two-year wait requirement.
webroot/src/pages/LicenseeDashboard/LicenseeDashboard.ts (1)
120-123: Computed flag wired — consistent with model exposureThis mirrors Licensee.hasEncumbranceLiftedWithinWaitPeriod and is used by the modal. Once the model is updated to include license-level checks, UI will reflect it automatically.
webroot/src/locales/es.json (1)
770-770: LGTM! Translation aligns well with English version.The Spanish translation accurately conveys the two-year wait period requirement for privilege purchases after encumbrance lifting. The message is clear and follows appropriate Spanish grammatical structure.
webroot/src/models/Licensee/Licensee.model.spec.ts (5)
92-92: LGTM! Test coverage for new method.The assertion correctly verifies that the new
hasEncumbranceLiftedWithinWaitPeriod()method returns false for default licensee instances.
201-201: LGTM! Test coverage maintained.The assertion correctly verifies that
hasEncumbranceLiftedWithinWaitPeriod()returns false for licensees created with specific values.
489-489: LGTM! Serializer test coverage.The test correctly verifies that
hasEncumbranceLiftedWithinWaitPeriod()returns false for licensees created through the serializer.
622-622: LGTM! Test for inactive military status scenario.The test correctly verifies that
hasEncumbranceLiftedWithinWaitPeriod()returns false when dealing with licensees having inactive military status.
683-707: LGTM! Comprehensive test for wait period logic.The test effectively validates the wait period functionality by:
- Creating mock privileges with different encumbrance lift statuses
- Mocking the underlying
isLatestEncumbranceWithinWaitPeriod()method appropriately- Testing the aggregation logic in
hasEncumbranceLiftedWithinWaitPeriod()The test correctly returns
truewhen at least one privilege has a recent encumbrance lift.webroot/src/models/License/License.model.ts (1)
9-9: LGTM! Consistent import structure.Adding
serverDatetimeFormatimport maintains consistency with existing date format imports and supports the new wait period functionality.webroot/src/models/License/License.model.spec.ts (7)
9-9: LGTM! Import addition supports new functionality.Adding
serverDatetimeFormatimport is necessary for the updated test data and ensures consistent date formatting in tests.
83-83: LGTM! Test coverage for new method.The assertion correctly verifies that the new
isLatestEncumbranceWithinWaitPeriod()method returns false for default license instances.
155-155: LGTM! Test coverage maintained.The test correctly verifies that
isLatestEncumbranceWithinWaitPeriod()returns false for licenses with custom values.
252-252: LGTM! Test for encumbered license.The test correctly verifies that
isLatestEncumbranceWithinWaitPeriod()returns false for an encumbered license without recent lift dates.
270-271: LGTM! Updated test data with proper date formatting.The test data correctly uses:
serverDatetimeFormatforcreationDate(consistent with the implementation)serverDateFormatforeffectiveLiftDate(maintaining existing format)The timing (6 months creation, 3 months lift) creates a realistic test scenario for wait period logic.
604-604: LGTM! Test validates wait period detection.The test correctly expects
isLatestEncumbranceWithinWaitPeriod()to return true for a privilege with an encumbrance lifted 3 months ago (within the 2-year wait period).
686-686: LGTM! Test for deactivated privilege.The test correctly verifies that deactivated privileges return false for the wait period check, ensuring the method only considers active encumbrance scenarios.
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
webroot/src/models/License/License.model.spec.ts (1)
193-198: Also assert serializer mapping in the license serializer testFor completeness, confirm both effectiveStartDate -> startDate and effectiveLiftDate -> endDate mappings in the license serializer path as well.
Apply this diff to add mapping assertions:
expect(license.adverseActions[0]).to.be.an.instanceof(AdverseAction); + // Mapping from server fields should populate model fields: + expect(license.adverseActions[0].startDate).to.equal(data.adverseActions[0].effectiveStartDate); + expect(license.adverseActions[0].endDate).to.equal(data.adverseActions[0].effectiveLiftDate);Also applies to: 218-220
🧹 Nitpick comments (1)
webroot/src/models/License/License.model.spec.ts (1)
793-807: Time-dependent edge cases can be flaky without a fixed “now”These assertions depend on the current date/time (moment()). Consider freezing time in tests to avoid flakiness around DST or midnight boundaries.
If you use sinon already in tests, you can do:
import sinon from 'sinon'; let clock: sinon.SinonFakeTimers; before(() => { clock = sinon.useFakeTimers(new Date('2025-08-01T12:00:00Z')); }); after(() => { clock.restore(); });Alternatively, derive all dates from a single const now = moment() within each test and build relative dates from that same reference.
Also applies to: 808-822
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
webroot/src/models/License/License.model.spec.ts(7 hunks)webroot/src/models/License/License.model.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- webroot/src/models/License/License.model.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: rmolinares
PR: csg-org/CompactConnect#1011
File: webroot/src/models/Licensee/Licensee.model.ts:276-279
Timestamp: 2025-08-13T20:28:33.143Z
Learning: The two-year encumbrance wait period feature only applies to privileges, not licenses. The logic should only check if privilege encumbrances that are no longer active have lift dates within the past two years.
🧬 Code Graph Analysis (1)
webroot/src/models/License/License.model.spec.ts (3)
webroot/src/app.config.ts (2)
serverDatetimeFormat(101-101)serverDateFormat(100-100)webroot/src/models/License/License.model.ts (1)
License(69-221)webroot/src/models/AdverseAction/AdverseAction.model.ts (1)
AdverseAction(33-98)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CheckWebroot
🔇 Additional comments (8)
webroot/src/models/License/License.model.spec.ts (8)
9-9: Importing serverDatetimeFormat for creationDate formatting is appropriateGood call pulling in serverDatetimeFormat for precise creationDate values in test payloads.
83-84: Baseline assertion for wait-period default looks goodAsserting false by default on a fresh License instance is correct and guards accidental regressions.
155-156: Serializer-based license scenario: assertion LGTMAffirming the new method returns false here is consistent with only active encumbrances present.
251-253: Active encumbrance serializer case validated correctlyWith only an active encumbrance, returning false for isLatestEncumbranceWithinWaitPeriod is correct.
769-776: Ensure the “mixed active and non-active” test truly includes an active encumbranceThe first encumbrance is intended to be active (no endDate + startDate in the past). After fixing the property name to startDate (see previous comment), the test will align with the intent. No additional changes needed beyond that rename.
604-605: Privilege serializer scenario correctly asserts wait-period = trueGood coverage for the positive case; aligns with the 2-year rule for privileges.
686-687: Deactivated privilege scenario looks goodEnsures deactivated status doesn’t falsely trigger the wait-period logic.
581-605: Confirm: 2‑year wait period is applied only to privileges — no change requiredVerified callers gate the wait-period to privileges. Summary:
- webroot/src/models/Licensee/Licensee.model.ts
- hasEncumbranceLiftedWithinWaitPeriod() → returns this.privileges?.some(privilege => privilege.isLatestEncumbranceWithinWaitPeriod()) (≈ line 276)
- purchase eligibility uses !this.hasEncumbranceLiftedWithinWaitPeriod() (≈ line 289)
- webroot/src/pages/LicenseeDashboard/LicenseeDashboard.vue
- UI uses v-if="hasEncumbranceLiftedWithinWaitPeriod" to show the message (≈ line 127)
- webroot/src/models/License/License.model.ts
- isLatestEncumbranceWithinWaitPeriod() is defined on License (≈ lines 198–219) — implementation is correct but is only invoked for privileges via Licensee
- Tests and server-side checks also reflect privilege-only enforcement:
- webroot/src/models/License/License.model.spec.ts (unit tests for the method)
- webroot/src/models/Licensee/Licensee.model.spec.ts (tests for privilege gating)
- backend purchase tests and attestations reference the two‑year rule for privileges
Conclusion: consumers already restrict the 2‑year wait period to privileges. No code change required.
jsandoval81
left a comment
jsandoval81
left a comment
There was a problem hiding this comment.
This looks really solid! 🚀
Just a small number of really minor items below.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
webroot/src/models/License/License.model.spec.ts (2)
689-804: Stabilize time-sensitive tests by freezing “now”These tests compute dynamic boundaries (e.g., exactly two years ago, just under two years) and compare against moment(). To reduce flakiness around DST/midnight edges, freeze the clock during this suite.
Example approach (outside the shown ranges; add to the describe() lifecycle):
// At top-level in this spec file: import sinon from 'sinon'; describe('License model', () => { let clock: sinon.SinonFakeTimers; before(() => { // existing setup... // Freeze to a safe midday time const fixedNow = new Date('2025-06-15T12:00:00Z').getTime(); clock = sinon.useFakeTimers({ now: fixedNow, toFake: ['Date'] }); // If you prefer not to pull sinon in this spec, you can also do: // (moment as any).now = () => fixedNow; }); after(() => { clock?.restore(); // If using moment.now override: // (moment as any).now = Date.now; }); });This ensures moment() in both production code and tests resolve to a fixed instant, making boundary checks deterministic.
694-706: Nit: align startDate with creationDate for readabilityThe second active encumbrance uses creationDate 2024-02-01 but startDate 2024-01-01. It’s still active, but mirroring the creationDate improves clarity.
new AdverseAction({ creationDate: '2024-02-01T00:00:00Z', - startDate: '2024-01-01', + startDate: '2024-02-01', endDate: null })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (4)
webroot/src/models/License/License.model.spec.ts(8 hunks)webroot/src/models/License/License.model.ts(1 hunks)webroot/src/models/Licensee/Licensee.model.spec.ts(5 hunks)webroot/src/models/Licensee/Licensee.model.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- webroot/src/models/Licensee/Licensee.model.spec.ts
- webroot/src/models/Licensee/Licensee.model.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: rmolinares
PR: csg-org/CompactConnect#1011
File: webroot/src/models/Licensee/Licensee.model.ts:276-279
Timestamp: 2025-08-13T20:28:33.143Z
Learning: The two-year encumbrance wait period feature only applies to privileges, not licenses. The logic should only check if privilege encumbrances that are no longer active have lift dates within the past two years.
📚 Learning: 2025-08-13T20:28:33.143Z
Learnt from: rmolinares
PR: csg-org/CompactConnect#1011
File: webroot/src/models/Licensee/Licensee.model.ts:276-279
Timestamp: 2025-08-13T20:28:33.143Z
Learning: The two-year encumbrance wait period feature only applies to privileges, not licenses. The logic should only check if privilege encumbrances that are no longer active have lift dates within the past two years.
Applied to files:
webroot/src/models/License/License.model.ts
📚 Learning: 2025-08-13T20:28:33.143Z
Learnt from: rmolinares
PR: csg-org/CompactConnect#1011
File: webroot/src/models/Licensee/Licensee.model.ts:276-279
Timestamp: 2025-08-13T20:28:33.143Z
Learning: When determining the latest encumbrance for wait period calculations, focus on the lift date (endDate) of inactive encumbrances rather than creation date, as the business requirement is based on when encumbrances were lifted.
Applied to files:
webroot/src/models/License/License.model.ts
📚 Learning: 2025-05-30T13:48:25.375Z
Learnt from: landonshumway-ia
PR: csg-org/CompactConnect#824
File: backend/compact-connect/lambdas/python/provider-data-v1/handlers/encumbrance.py:116-201
Timestamp: 2025-05-30T13:48:25.375Z
Learning: In encumbrance handling code, prefer to keep privilege and license encumbrance lifting implementations decoupled rather than extracting shared logic, as requirements between these implementations are likely to change in the future.
Applied to files:
webroot/src/models/License/License.model.ts
🧬 Code Graph Analysis (2)
webroot/src/models/License/License.model.ts (2)
webroot/src/models/AdverseAction/AdverseAction.model.ts (1)
AdverseAction(33-98)webroot/src/app.config.ts (1)
serverDateFormat(100-100)
webroot/src/models/License/License.model.spec.ts (3)
webroot/src/app.config.ts (2)
serverDatetimeFormat(101-101)serverDateFormat(100-100)webroot/src/models/License/License.model.ts (1)
License(69-220)webroot/src/models/AdverseAction/AdverseAction.model.ts (1)
AdverseAction(33-98)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CheckWebroot
🔇 Additional comments (3)
webroot/src/models/License/License.model.spec.ts (3)
9-9: LGTM: adds serverDatetimeFormat import for consistent test data formattingImporting serverDatetimeFormat aligns test fixtures with app config and avoids hardcoding formats.
83-83: LGTM: assertions for wait-period behavior across scenariosThe added expectations validate default, explicit, serializer, privilege, and deactivated cases for the wait-period logic and match the intended semantics.
Also applies to: 155-155, 252-252, 605-606, 687-688
270-271: LGTM: serializer mapping assertions and creationDate formatting
- Using serverDatetimeFormat for creationDate is consistent with model expectations.
- Asserting endDate equals effectiveLiftDate protects against regressions in AdverseActionSerializer mapping.
Also applies to: 579-580
|
@jlkravitz This is ready for your review. |
jlkravitz
left a comment
jlkravitz
left a comment
There was a problem hiding this comment.
one nit! looks good otherwise.
jlkravitz
left a comment
jlkravitz
left a comment
There was a problem hiding this comment.
@isabeleliassen Good to merge!
4 participants
Requirements List
Description List
Testing List
yarn test:unit:allshould run without errors or warningsyarn serveshould run without errors or warningsyarn buildshould run without errors or warningsgetAuthenticatedLicenseeUserinmock.data.apito returnlicensees.providers[1]Closes #950
Summary by CodeRabbit
New Features
Tests
Chores