-
Notifications
You must be signed in to change notification settings - Fork 80
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
fix #2091 #2095
fix #2091 #2095
Conversation
if (a.visibility == 'public' or a.study.has_access(user)): | ||
return True | ||
else: | ||
for c in a.descendants.nodes(): |
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 don't understand how this fits in...?
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 was there before and decided to leave it. Basically, it protects for the case that an artifact is not public but one of its children is. As you know, this could happen right now and we have an issue to solve it and it doesn't hurt. BTW and AFAIK this is still possible in the test db so it's being tested.
The travis failure is the stochastic error we see ...
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 are right, it's not shown here ... but it does exist in the original code: https://github.com/biocore/qiita/blob/master/qiita_db/meta_util.py#L131, perhaps github doing some magic?
👍 short of the question, and the present travis failures |
It was? I didn't see it on review hence the question
…On Mar 29, 2017 08:31, "Antonio Gonzalez" ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In qiita_db/meta_util.py
<#2095 (comment)>:
> else:
- for c in a.descendants.nodes():
- if (c.visibility == 'public' or c.study.has_access(user)):
- return True
+ if (a.visibility == 'public' or a.study.has_access(user)):
+ return True
+ else:
+ for c in a.descendants.nodes():
This was there before and decided to leave it. Basically, it protects for
the case that an artifact is not public but one of its children is. As you
know, this could happen right now and we have an issue to solve it and it
doesn't hurt. BTW and AFAIK this is still possible in the test db so it's
being tested.
The travis failure is the stochastic error we see ...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2095 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAc8ssVOz--PkMvg6wd4M1XfXSQVi-vHks5rqnlQgaJpZM4Ms-oU>
.
|
Changes Unknown when pulling 31dae8d on antgonza:fix-2091 into ** on biocore:master**. |
@wasade, @josenavas, could you review?