Skip to content

5001 BCCR remote errors - #5106

Open
gdalcengio wants to merge 2 commits into
developfrom
5001-BCCR-remote-errors
Open

5001 BCCR remote errors#5106
gdalcengio wants to merge 2 commits into
developfrom
5001-BCCR-remote-errors

Conversation

@gdalcengio

Copy link
Copy Markdown
Contributor
  • feat: display remote BCCR endpoint error and update endpoint to match
  • test: update BCCR account_id tests and display tests

@gdalcengio
gdalcengio force-pushed the 5001-BCCR-remote-errors branch from a3e320a to 6e10023 Compare August 18, 2026 21:17

@pbastia pbastia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A couple of nitpicks, looks great! Approved once you've added the tiny changes

try:
account_details = bccr_service.get_account_details(account_id=account_id)
trading_name = getattr(account_details, "trading_name", None) if account_details else None
return 200, {"bccr_trading_name": trading_name, "has_remote_bccr_errors": False}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
return 200, {"bccr_trading_name": trading_name, "has_remote_bccr_errors": False}
return 200, {"bccr_trading_name": trading_name}

We can probably omit that flag, it's already optional on the frontend. Just less code!


@router.get(
"/bccr/accounts/{account_id}/compliance-report-versions/{compliance_report_version_id}",
response={200: dict, custom_codes_4xx: Message},

@pbastia pbastia Aug 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should use this PR to specify a ninja schema, it's missing:

class BCCRAccountDetailsSchema(Schema):
    bccr_trading_name: str
    has_remote_bccr_errors: Optional[bool]

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.

2 participants