Skip to content

Update trade compliance notifications with more accurate status#506

Merged
lukaszgryglicki merged 3 commits into
mainfrom
feature/trade-compliance-notifications
Jul 22, 2026
Merged

Update trade compliance notifications with more accurate status#506
lukaszgryglicki merged 3 commits into
mainfrom
feature/trade-compliance-notifications

Conversation

@mkdolan

@mkdolan mkdolan commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Update trade compliance notification messaging for more accurate status

Test plan

  • Verify trade compliance notifications display in corporate dashboard correctly

Signed-off-by: Mike Dolan <mikedolan@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Updates corporate dashboard trade-compliance notifications with broader, more accurate CLA terminology.

Changes:

  • Renames sanctions-screening status messages.
  • Replaces ECLA-specific wording with CLA terminology.
  • Updates request failure messaging.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ddc0bda5-097e-49ad-81e6-e15da9df66bd

📥 Commits

Reviewing files that changed from the base of the PR and between 8988b09 and 1995767.

📒 Files selected for processing (1)
  • src/app/modules/corporate-contributor/container/corporate-dashboard/corporate-dashboard.component.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/app/modules/corporate-contributor/container/corporate-dashboard/corporate-dashboard.component.ts

Walkthrough

The corporate dashboard updates trade-compliance warning messaging for sanctioned organizations and employee signature errors. Non-sanctioned signature request errors now check the server error before using the generic fallback.

Changes

Trade compliance messaging

Layer / File(s) Summary
Dashboard compliance warnings and request errors
src/app/modules/corporate-contributor/container/corporate-dashboard/corporate-dashboard.component.ts
Organization setup and employee signature compliance paths use updated “Compliance Review Required” messaging. Signature request errors also check response.errors.server after response.errors.project_id before the generic fallback.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the main change: revised trade-compliance notification messaging for sanctioned cases.
Description check ✅ Passed The description is clearly related to the PR and summarizes the updated trade-compliance notifications and test plan.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/trade-compliance-notifications

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.


Comment @coderabbitai help to get the list of available commands.

@lukaszgryglicki

Copy link
Copy Markdown
Member

cc @mlehotskylf @ahmedomosanya

@lukaszgryglicki

Copy link
Copy Markdown
Member

I'm not a Frontend specialist, I've asked AI to address Copilot's feedback and this is what I've got cc @mlehotskylf @ahmedomosanya (Ahmed can you PTAL ?):

The wording in this PR is correct and reflects the intended distinction: a screening flag requires additional trade-compliance review; it is not itself a sanctions determination.

A few small frontend corrections are needed so the approved wording is always shown consistently:

@copilot please:

  1. In both branches that handle response.errors.sanctioned, remove response.errors.description || and assign the new compliance-review message directly.

  2. Use Compliance Review Required as the title in both branches. Please replace Review Required in the second branch.

  3. Update the nearby comments that say the organization “is sanctioned” so they instead say it requires additional trade-compliance review. Keep the API field name sanctioned unchanged for compatibility.

Please make no other behavioral changes.

Acceptance check: when either endpoint returns errors.sanctioned, including when the response also contains errors.description, the modal must always display:

Title:
Compliance Review Required

Message:
We're sorry, but this organization requires additional trade compliance review, so the Contributor License Agreement (CLA) cannot be completed at this time. If you believe this is an error, please contact EasyCLA Support via the chat widget.

Backend follow-up: 5113. It aligns the API response wording. This frontend PR can merge independently.

…er description fallback

- Remove `response.errors.description ||` from both sanctioned branches so
  the compliance-review message is always shown, even when the server response
  includes errors.description
- Use 'Compliance Review Required' as title in both branches (was 'Review Required'
  in postEmployeeSignatureRequest path)
- Update inline comments to say 'requires additional trade-compliance review'
  instead of 'is sanctioned'
Copilot AI review requested due to automatic review settings July 21, 2026 07:23

Copilot AI commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

I'm not a Frontend specialist, I've asked AI to address Copilot's feedback and this is what I've got cc @mlehotskylf @ahmedomosanya (Ahmed can you PTAL ?):

The wording in this PR is correct and ref...

Done in the latest commit. Both sanctioned branches now:

  1. Assign the compliance-review message directly (removed response.errors.description || from both paths)
  2. Use Compliance Review Required as the title
  3. Have updated comments referencing "trade-compliance review" instead of "sanctioned"

No other behavioral changes were made.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/app/modules/corporate-contributor/container/corporate-dashboard/corporate-dashboard.component.ts (1)

201-203: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add regression coverage for the compliance-message paths.

Please verify or add component tests covering the persisted sanctioned warning and both response.errors.sanctioned branches. Assert the exact title/message, ensure a server-provided description is ignored, and verify the non-sanctioned fallback order: project_id, then server, then the generic message.

Also applies to: 241-245, 301-317

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/app/modules/corporate-contributor/container/corporate-dashboard/corporate-dashboard.component.ts`
around lines 201 - 203, Add regression tests for the corporate dashboard
component’s compliance-message handling, covering persisted sanctioned warnings
and both response.errors.sanctioned branches. Assert the exact title and
message, confirm server-provided description is ignored, and verify
non-sanctioned fallback precedence from project_id to server to the generic
message.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@src/app/modules/corporate-contributor/container/corporate-dashboard/corporate-dashboard.component.ts`:
- Around line 201-203: Add regression tests for the corporate dashboard
component’s compliance-message handling, covering persisted sanctioned warnings
and both response.errors.sanctioned branches. Assert the exact title and
message, confirm server-provided description is ignored, and verify
non-sanctioned fallback precedence from project_id to server to the generic
message.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1f7f9577-0c99-4cfb-a775-0ef094bdf30a

📥 Commits

Reviewing files that changed from the base of the PR and between 5aad4df and 8988b09.

📒 Files selected for processing (1)
  • src/app/modules/corporate-contributor/container/corporate-dashboard/corporate-dashboard.component.ts

@lukaszgryglicki

Copy link
Copy Markdown
Member

PTAL — the Copilot follow-up now addresses the frontend review points:

  • both blocked employee-signature paths always display the approved compliance-review message directly;
  • both paths use Compliance Review Required;
  • a backend-provided description can no longer override the approved wording;
  • the existing errors.sanctioned API property remains only as the machine-readable compatibility signal.

No further frontend changes are needed from my side.

Backend wording and the existing 403 classification are being aligned in linuxfoundation/easycla#5113. This frontend PR can merge independently.

Yarn dependency audit is not related to this PR. cc @ahmedomosanya @mlehotskylf @mkdolan

@lukaszgryglicki

Copy link
Copy Markdown
Member

@copilot Please make one final comment-only wording correction in corporate-dashboard.component.ts.

In getOrganizationInformation(), replace:

// currently marked sanctioned, show a non-blocking WARNING but still proceed to the

with:

// currently marked for additional trade-compliance review, show a non-blocking WARNING but still proceed to the

Keep the existing identifiers and API keys, including isSanctioned and errors.sanctioned, unchanged for compatibility.

Please make no other code, behavior, title, message, or test changes.

Copilot AI review requested due to automatic review settings July 21, 2026 09:02

Copilot AI commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

@copilot Please make one final comment-only wording correction in corporate-dashboard.component.ts.

In getOrganizationInformation(), replace:
...

Addressed in commit 1995767. Updated the inline comment in getOrganizationInformation() to say “currently marked for additional trade-compliance review…” while keeping isSanctioned and errors.sanctioned unchanged.

@lukaszgryglicki

Copy link
Copy Markdown
Member

Both trade-compliance block paths now:

  • use Compliance Review Required;
  • display the approved compliance-review message directly;
  • prevent a backend-provided description from overriding that message; and
  • use trade-compliance-review terminology in the accompanying comments.

The existing errors.sanctioned field remains unchanged solely as the API compatibility signal.

Backend wording and the existing 403 handling are aligned in linuxfoundation/easycla#5113. linuxfoundation/easycla#5108 is superseded by that PR.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@lukaszgryglicki

lukaszgryglicki commented Jul 21, 2026

Copy link
Copy Markdown
Member

/lgtm

@ahmedomosanya @mkdolan @mlehotskylf this is ready for merge IMHO.

@lukaszgryglicki
lukaszgryglicki merged commit 0617bfe into main Jul 22, 2026
13 of 14 checks passed
@lukaszgryglicki
lukaszgryglicki deleted the feature/trade-compliance-notifications branch July 22, 2026 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants