Share analyses #1758
Share analyses #1758
Conversation
squirrelo
force-pushed
the
share-analyses
branch
from
528e31a to
5fb1147
Compare
squirrelo
force-pushed
the
share-analyses
branch
from
7c757a8 to
2314e24
Compare
|
Conflicts, please solve. |
|
done, ready for review. |
|
Changes Unknown when pulling b40bb8b on squirrelo:share-analyses into * on biocore:master*. |
qiita_db/analysis.py
Outdated
| if user.id in self.shared_with: | ||
| return | ||
| # Make sure the analysis is not already shared with the given user | ||
| if user.id == self.owner: |
There was a problem hiding this comment.
I think you are missing and user.id in self.shared_with, right?
|
A few comments. |
|
Comments addressed |
| selected = self.get_argument('selected', None) | ||
| deselected = self.get_argument('deselected', None) | ||
|
|
||
| if selected is not None: |
There was a problem hiding this comment.
What happens if both are None?
There was a problem hiding this comment.
No share or unshare actions, but the user email links and user list is still returned. This is leveraged for initial population of the shared list in the page.
|
A couple of comments |
|
Comments addressed |
| <script type="text/javascript"> | ||
| $(document).ready(function () { | ||
| current_id = {{analysis_id}}; | ||
| update_share('analysis'); |
There was a problem hiding this comment.
Pass the current_id as a parameter.
There was a problem hiding this comment.
This does not work for the on select and deselect that are in the sharing.js
There was a problem hiding this comment.
Can you elaborate? Are you saying that you can't modify the functions that you created?
There was a problem hiding this comment.
No, I'm saying that the calls on line 21 and 25 of sharing.js have no idea what the current_id is if it is not a global variable, so this suggestion will not work.
There was a problem hiding this comment.
I don't see current_id used in those lines.
There was a problem hiding this comment.
Exactly. There is no way of passing in current_id there as a parameter if it is not a global variable since it is unknown, so modifying the function will not work. I have a way of using data variables I am testing right now.
| current_study = {{study_info['study_id']}}; | ||
| update_share(); | ||
| current_id = {{study_info['study_id']}}; | ||
| update_share('study'); |
There was a problem hiding this comment.
Same here and fix indentation.
squirrelo
force-pushed
the
share-analyses
branch
from
b59227d to
5ad25f0
Compare
|
Now uses data attribute instead of global. |
| @@ -1,5 +1,3 @@ | |||
| var current_study = null; | |||
|
|
|||
| $(document).ready(function () { | |||
There was a problem hiding this comment.
Actually I just saw this - this is a library, why does it has a $(document).ready call? It seems weird that this is in the library and this may be the source of the difficulties that you where finding. If this is used to initialize the different parts of this library, it may be useful to actually have a sharing_init call that you call in the $(document).ready of the actual HTML pages that is using this library.
There was a problem hiding this comment.
Because you can have as many document.ready calls in a single page as needed, and this also standardizes naming and initialization of the sharing dropdown for use in the other helper functions.
|
Closed to merge onto portals branch. |
4 participants
Fixes #1615
Requires #1755
This adds the ability to share analyses to the completed analysis page. It also adds messaging to sharing, so users will receive a message when a study or analysis is shared or unshared with them.