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

fix: updated transfer labels for BCeID users according to new Act #2722

Merged

Conversation

hamed-valiollahi
Copy link
Collaborator

This commit makes the following updates to the New Transfer page for BCeID users, ensuring alignment with the new Act:

  • Change the dropdown label from 'Select a Fuel Supplier' to 'Select an Organization'.

  • The signing authority declaration statement is now updated to: 'I confirm that records evidencing each matter reported under section 17 of the Low Carbon Fuel (General) Regulation are available on request.'

Additionally, a database migration is added to apply the signing authority declaration statement label change within the existing record.

Closes #2690

This commit makes the following updates to the New Transfer page for BCeID users, ensuring alignment with the new Act:

- Change the dropdown label from 'Select a Fuel Supplier' to 'Select an Organization'.

- The signing authority declaration statement is now updated to: 'I confirm that records evidencing each matter reported under section 17 of the Low Carbon Fuel (General) Regulation are available on request.'

Additionally, a database migration is added to apply the signing authority declaration statement label change within the existing record.

Closes #2690
@hamed-valiollahi hamed-valiollahi force-pushed the fix/hamed-transfer-act-label-changes-2690 branch from 5c68f75 to dcfb8d3 Compare November 6, 2023 18:32
"""
signing_authority_assertion = apps.get_model('api', 'SigningAuthorityAssertion')
try:
assertion = signing_authority_assertion.objects.get(id=1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we certain that id=1 is the correct assertion here? Is there a more consistent way to get the correct assertion across environments?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You are right, Alex! Using record IDs isn't a good practice because they can change in different environments. I was unable to find any unique names for the SigningAuthorityAssertion records in the code. Would you like me to add a unique name to each record? Then I could update this migration file to use these unique names instead of the automatically created IDs.

Another solution would be to find the records by their text (description). This isn't ideal, but it is still better than updating the records by their IDs.

Copy link
Collaborator

@AlexZorkin AlexZorkin Nov 9, 2023

Choose a reason for hiding this comment

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

hmm yeah a bit tricky if there isn't already a different unique field, and selecting by long form text isn't ideal. Let's just continue with your id solution but please confirm that each environment uses those ids for our target records.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can confirm that all the environments (test, development, and production) use the same IDs for our target records.

@AlexZorkin AlexZorkin merged commit f9b2039 into main-release-jan-2024 Nov 15, 2023
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