Skip to content

PSP-11387 remove the notice of claim if the noc comment/date are not specified.#5286

Merged
devinleighsmith merged 3 commits intobcgov:devfrom
devinleighsmith:psp-11387
Apr 20, 2026
Merged

PSP-11387 remove the notice of claim if the noc comment/date are not specified.#5286
devinleighsmith merged 3 commits intobcgov:devfrom
devinleighsmith:psp-11387

Conversation

@devinleighsmith
Copy link
Copy Markdown
Collaborator

No description provided.

@devinleighsmith devinleighsmith self-assigned this Apr 17, 2026
@devinleighsmith devinleighsmith added bug Something isn't working 6.2 labels Apr 17, 2026
@github-actions
Copy link
Copy Markdown
Contributor

See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/5286

@asanchezr asanchezr changed the title remove the notice of claim if the noc comment/date are not specified. PSP-11387 remove the notice of claim if the noc comment/date are not specified. Apr 18, 2026
Copy link
Copy Markdown
Collaborator

@asanchezr asanchezr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. See comments

product: null,
project: null,
noticeOfClaim: exists(this.noticeOfClaim) ? [this.noticeOfClaim] : [],
noticeOfClaim: hasNoticeOfClaim && noticeOfClaim ? [noticeOfClaim] : [],
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
noticeOfClaim: hasNoticeOfClaim && noticeOfClaim ? [noticeOfClaim] : [],
noticeOfClaim: hasNoticeOfClaim && exists(noticeOfClaim) ? [noticeOfClaim] : [],

toApi(): ApiGen_Concepts_AcquisitionFile {
const fileProperties = this.properties.map(x => this.toPropertyApi(x));
const sortedProperties = applyDisplayOrder(fileProperties);
const noticeOfClaim: ApiGen_Concepts_NoticeOfClaim | null = this.noticeOfClaim
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: use exists()

noticeOfClaim: ApiGen_Concepts_NoticeOfClaim;

toApi(): ApiGen_Concepts_AcquisitionFile {
const noticeOfClaim: ApiGen_Concepts_NoticeOfClaim | null = this.noticeOfClaim
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: same comments regarding exists()

comment: yup.string().max(4000, 'Notice of claim comment must be at most ${max} characters'),
comment: yup
.string()
.nullable()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this nullable validation should be added to AddAcquisitionFileYupSchema.ts as well - right?

const sortedProperties = applyDisplayOrder(fileProperties);
const personId = this.responsiblePayer?.personId ?? null;
const organizationId = !personId ? this.responsiblePayer?.organizationId ?? null : null;
const noticeOfClaim: ApiGen_Concepts_NoticeOfClaim | null = this.noticeOfClaim
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comments re: exists()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also AddManagementFormYupSchema needs to be updated with nullable comment field

@sonarqubecloud
Copy link
Copy Markdown

@github-actions
Copy link
Copy Markdown
Contributor

See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/5286

@devinleighsmith devinleighsmith added this pull request to the merge queue Apr 20, 2026
Merged via the queue into bcgov:dev with commit 48afff3 Apr 20, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.2 bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants