-
Couldn't load subscription status.
- Fork 79
Refactoring the refactor (II): Add artifact creation page #1680
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
Refactoring the refactor (II): Add artifact creation page #1680
Conversation
…tudy-pages-artifact-creation
… into artifact-study-pages-artifact-creation
|
This is ready for review |
| User adding the atrifact | ||
| filepaths : dict of {str: [str, ...], ...} | ||
| List of files to attach to the artifact, keyed to the file type | ||
| filepaths : dict of {str: str} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure the description and the example match.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch - done
|
Some comments. |
| User adding the atrifact | ||
| filepaths : dict of {str: [str, ...], ...} | ||
| List of files to attach to the artifact, keyed to the file type | ||
| filepaths : str |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This description doesn't match "my inferred type", it seems like it really is a dictionary, not a string, am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🍺 you're right
|
Just a few comments. |
|
👍 @ElDeveloper can you merge? |
…-creation Refactoring the refactor (II): Add artifact creation page
This depends on #1679, so review/merge that one first
This PR adds the page and code needed for adding an artifact to the system. At this point, this page is not integrated with the rest of the system in order to keep the PR small. It will be integrated in the next PR, which I'm almost done.
The tests, however, should pass.
The previous implementation was buggy and it did not allow to upload an artifact with more than a single lane or if it was missing the reverse reads. The user feedback has been also improved by performing the files checks on the fly rather when it clicks the create button.