Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Task/one reg purpose only #2460

Merged
merged 21 commits into from
Nov 21, 2024
Merged

Task/one reg purpose only #2460

merged 21 commits into from
Nov 21, 2024

Conversation

andrea-williams
Copy link
Contributor

@andrea-williams andrea-williams commented Nov 12, 2024

Changes made:

  • Operation records allow a maximum of one registration_purpose, which is now an attribute on the Operation model
  • the RegistrationPurpose model has been deleted (no longer necessary)
  • all python unit tests have been updated for the new location of registration_purpose (232 unit tests were failing to begin with, it was a big refactor)
  • edited front-end code so that the Registration Purpose field only expects one string, not an array of strings (there will be more work required for this form field, but I'll address that in my next PR for my actual ticket)
  • updated Registration wiki to describe new registration purpose behaviour

@andrea-williams andrea-williams force-pushed the task/one-reg-purpose-only branch 5 times, most recently from 9a87670 to 3f1be37 Compare November 14, 2024 20:01
@andrea-williams andrea-williams marked this pull request as ready for review November 14, 2024 20:47
@andrea-williams andrea-williams force-pushed the task/one-reg-purpose-only branch 5 times, most recently from e5738e1 to e47f08c Compare November 19, 2024 19:59
Copy link
Contributor

@BCerki BCerki left a comment

Choose a reason for hiding this comment

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

Nice work on this, it was huge! Some line notes in the PR, and one overall comment: The reg 1 tests shouldn't have needed updating because none of our changes at this point should affect reg 1 (unless we're doing custom migration stuff). I think the cause is adding registration_purpose into a v1 schema.

bc_obps/registration/api/v1/operations.py Outdated Show resolved Hide resolved
bc_obps/registration/api/v1/operations.py Outdated Show resolved Hide resolved
bc_obps/registration/api/v1/operations.py Outdated Show resolved Hide resolved
bc_obps/registration/models/operation.py Show resolved Hide resolved
bc_obps/registration/schema/v1/operation.py Outdated Show resolved Hide resolved
bc_obps/service/operation_service_v2.py Show resolved Hide resolved
bc_obps/service/operation_service_v2.py Show resolved Hide resolved
@andrea-williams andrea-williams force-pushed the task/one-reg-purpose-only branch from b8dc72e to 38e6723 Compare November 20, 2024 01:53
Copy link
Contributor

@ayeshmcg ayeshmcg left a comment

Choose a reason for hiding this comment

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

@andrea-williams
From the reporting perspective, the service looks good—currently, only one registration purpose is returned. However, one enhancement needs to be made in AdditionalReportingData.tsx.

Specifically, consider the following change:

  const registrationPurpose = (await getRegistrationPurpose(versionId))
    ?.registration_purpose;
  const includeElectricityGenerated =
    registrationPurpose === "OBPS Regulated Operation";

This will ensure that the includeElectricityGenerated property is determined correctly based on the registrationPurpose. Let me know if additional clarifications are needed.

@andrea-williams andrea-williams force-pushed the task/one-reg-purpose-only branch from 38e6723 to dfa54f7 Compare November 21, 2024 00:09
Copy link
Contributor

@BCerki BCerki left a comment

Choose a reason for hiding this comment

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

🤸

@andrea-williams andrea-williams force-pushed the task/one-reg-purpose-only branch from 653be95 to 4be1ece Compare November 21, 2024 22:27
@andrea-williams andrea-williams merged commit 10e2488 into develop Nov 21, 2024
40 checks passed
@andrea-williams andrea-williams deleted the task/one-reg-purpose-only branch November 21, 2024 22:45
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.

3 participants