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

CSL-24: Company Registration Certificate upload #25

Open
wants to merge 16 commits into
base: feature/csl-4-evidence
Choose a base branch
from

Conversation

jamiecarterHO
Copy link
Contributor

What?

CSL-24 Adds the configuration and files required to display the "Upload company registration certificate" page and enable uploads via file-vault to S3.

Why?

The updates will allow deployment of a file-vault container in the namespace that can connect to the S3 bucket, and add the page and functionality for uploads

Check list

  • I have reviewed my own pull request
  • I have written tests (if relevant)

@jamiecarterHO jamiecarterHO marked this pull request as draft November 29, 2024 11:28
@jamiecarterHO jamiecarterHO marked this pull request as ready for review November 29, 2024 15:52
@jamiecarterHO
Copy link
Contributor Author

Ready for first review. Some discussion to be had about where reusable methods/behaviours should be stored. I believe they can be shared cross-forms as they are but happy to entertain any alternative ideas @dk4g

@@ -55,9 +55,11 @@ module.exports = {
},
keycloak: {
token: process.env.KEYCLOAK_TOKEN_URL,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it is worth renaming this property to something like tokenUrl, otherwise it gives the impression that the token already exists.

async auth() {
const requiredProperties = ['clientId', 'secret', 'username', 'password'];

if (!config.keycloak.token) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

tokenUrl See comment about property naming in config.js

Comment on lines +25 to +26
* Assuming config.dateLocales is 'en-GB' and config.dateFormat is { day: '2-digit', month: '2-digit', year: 'numeric' }
* formatDate('2023-10-01'); // returns '01 10 2023'
Copy link
Collaborator

Choose a reason for hiding this comment

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

example can be updated to be more relevant to existing configuration data

*
* @example
* sanitiseFilename('filename.txt');
* // returns 'fi**REDACTED**le.txt'
Copy link
Collaborator

Choose a reason for hiding this comment

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

le.txt-> me.txt
* // returns 'fi**REDACTED**me.txt'

"add-another-substance": "Add another substance"
}
"add-another-substance": "Add another substance",
"delete": "Remove"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I noticed there is a remove translation on line 14, which probably can be reused? but will require updates on template and js event handlers

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