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

DownloadLink checks Checksum, FileUpload displays Limit #213

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

tomlovesgithub
Copy link
Contributor

@tomlovesgithub tomlovesgithub commented Aug 6, 2024

FileUpload component now displays the accepted types and file size limit, as well as preventing uploads when the scanner status is offline.

I have also amended the way these props are accepted and used.

DownloadLink is now ready for checksums, altho there is a bypass which means that this component can be rolled out prior to checksum creation (thanks for the idea @warrensearle).

@tomlovesgithub tomlovesgithub changed the title DownloadLink checks Checksum DownloadLink checks Checksum, FileUpload displays Limit Aug 14, 2024
@tomlovesgithub tomlovesgithub marked this pull request as ready for review August 15, 2024 15:27
Copy link
Member

@warrensearle warrensearle left a comment

Choose a reason for hiding this comment

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

Let's get this merged... though please see code comment/suggestion

Comment on lines +56 to +58
...mapGetters({
checksumsEnabled: 'candidateSettings/getUploadStatus', // Get checksumsEnabled from Vuex store
}),
Copy link
Member

Choose a reason for hiding this comment

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

This component probably shouldn't access the store directly but instead have a checksumsEnabled prop fed into it...
However if that would be a headache to roll out everywhere this component is used then let's stick with the current approach which nicely keeps everything in one place (just means we need the same store to exist in our apps that use this component) 🤓

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.

5 participants