-
Notifications
You must be signed in to change notification settings - Fork 79
Disable make processed data private #1142
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
Disable make processed data private #1142
Conversation
|
Should it fully solve #1128 before is merged? If not, why? |
|
I can work extend this and just make a single PR, I was thinking on just making multiple, smaller PR. |
|
If that's the plan, should this be a new branch? |
|
IMOO is not leaving the master in an inconsistent state, but I can create it's own branch. What do you prefer, a single PR with all of it or 3 small PR in its own branch? |
|
I prefer 1 single PR, if not tooo complex. |
|
Thanks! Could you add test for the newly added code in: remove_add_prep_template, display_template |
|
Yup, @josenavas and I chatted and we agreed that adding those tests are a full refactor. Thus, 👍 |
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.
these values are in the __init__.py file, so you can import directly from qiita_db.metadata_template
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! Fixed!
|
Juts a minor comment, but 👍 |
…e-make-processed-data-public
|
Changes Unknown when pulling 919bd88 on josenavas:disable-make-processed-data-public into * on biocore:master*. |
|
@adamrp any remaining comment? |
|
I have 2 👍 @adamrp can you merge if your are ok with the code? Thanks! |
|
AARGGGGH!!! I though this PR was merged!!! This is pretty bad as my current PR modifies some of the pages modified here! Can I get this merged so I can try to solve the potential hell of merge conflicts and complete my work? I'm surprised that I don't have any merge conflict on this one... I'll be working on this tomorrow morning and this is kind of blocking further development on my other issue. @antgonza or @adamrp can you merge? |
…ublic Disable make processed data private
|
Thanks @ElDeveloper |
Built on top of #1141
If the user is missing some of the columns that are required for our system, we don't allow it to request making the data private, since that's not possible until the missing columns are added. The system shows a message with the missing columns.
Fixes partially #1128