-
Couldn't load subscription status.
- Fork 79
Refactoring the refactor (I): Fix prep info creation #1679
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 (I): Fix prep info creation #1679
Conversation
… into artifact-study-pages-fix-prep-creation
… into artifact-study-pages-fix-prep-creation
| Returns | ||
| ------- | ||
| (list of str, list of str, dict of {str: list of str}) | ||
| The list of txt,csv files in the upload dir for the given study |
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.
csv is not in the list below.
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.
Yeah, same comment, plus txt,csv -> txt, csv
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.
I think this is actually a typo in the documentation. We are supporting tsv (Tab Separated Values) rather than csv (Comma Separated Values).
|
One minor comment, then 👍 @squirrelo or @ElDeveloper could you review? |
|
|
||
|
|
||
| def new_prep_template_get_req(study_id): | ||
| """ |
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.
Can you add a description to this function?
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.
Done
|
👍 Just a few comments, looking good overall! |
|
Comments addressed |
|
👍 |
…-creation Refactoring the refactor (I): Fix prep info creation
In the previous implementation, the addition of the prep info and the first artifact was forced to occur at the same point. We used that strategy at the beginning of Qiita and we changed it later because that did not follow the workflow.
At the same time, if the artifact creation fail for whatever reason, the entire page was broke and the user was not able to see the study page again.
Here, the addition of the artifact is removed from the addition of the prep information.
At this point, artifacts can't be added, but that will be added in the next PR, to keep them small.
Thus, the interface is broken at this point BUT the tests should pass.