Smarter access logic for reports' experiments
This MR takes advantage of the experiment.accessibility_for(user) method on the server to generate an access map to properly check which experiments the user has access to, and which they don't. Depending on the access map, the reports app will show either the full experiment name and give the corresponding URL, or give just the alias and no URL.
Closes #525
Merge request reports
Activity
mentioned in merge request !287 (closed)
@flavio.tarsetti do you think this solves #525?
assigned to @flavio.tarsetti
989 991 "status": "published", 990 992 "creation_date": self.report.creation_date.isoformat(), 991 993 "publication_date": self.report.publication_date.isoformat(), 994 "experiment_access_map": [False], This is a new field in the API (the "access map" I mentioned in the MR summary). I think the commit had some info on it but yeah I'll collate it here:
experiment_access_mapis a new field that can be returned by the report serializer. It is a list of boolean values. Each boolean value is related with the experiment of the same index:["exp1", "exp2", "exp3"] [True, False, True]"exp1" and "exp3" have
True, which means that "exp1" & "exp3" are visible to the user that is viewing this report."exp2" has
False, which means that "exp2" is not visible to the user.This map is made on the API side because it's trivial to do so, with the
experiment.accessibility_for()method available. Constructing such an access map on the client side would be much more difficult. So I figured it was worth adding a simple new field to the serializer.
@jdiefenbaugh I made some feedback in issue #525 .
added 1 commit
- 7cb0b083 - [reports][js] let user know why they cant see exps names
@samuel.gaist did you change beat.cmdline anytime recently? Because the pipelines are now failing for beat.web MRs...the "test_draw" beat.cmdline tests are failing
See https://gitlab.idiap.ch/beat/beat.web/-/jobs/167896, but the failures are the same for each MR
@jdiefenbaugh please define recently.
From the looks of it, it's unrelated to beat/beat.cmdline> itself directly. There are dependencies missing for graphviz. So it's likely a conda issue not pulling the necessary libraries.
@samuel.gaist actually the CI was fixed by andre yesterday so it's fine now!
added 11 commits
-
7cb0b083...8d495b91 - 8 commits from branch
master - 62dfb964 - [reports] smarter access logic for reports' experiments
- 3c440648 - [reports] fix nosetests to expect new field
- f64b18ed - [reports][js] let user know why they cant see exps names
Toggle commit list-
7cb0b083...8d495b91 - 8 commits from branch
added 9 commits
-
f64b18ed...aa723b6c - 6 commits from branch
master - 6e669419 - [reports] smarter access logic for reports' experiments
- 58c882f2 - [reports] fix nosetests to expect new field
- 29dc178a - [reports][js] let user know why they cant see exps names
Toggle commit list-
f64b18ed...aa723b6c - 6 commits from branch
@flavio.tarsetti @jdiefenbaugh It seems to be working as intended but there is something fishy. This is what I see if I look at the report without signing in. I will send you the report directly to check the status of the experiment that is shared with you as well.
Now I have two groups and as I saw earlier for !294 (merged) I should be able to see the "Experiment list" that contains all the experiments in the report. You cannot see it here in this report with two groups, but the links are ok!
Edited by Zohreh MOSTAANI-
@zmostaani : Sorry !294 (merged) and !288 (merged) have different fixes that are not merged, so you won't see it unless they are merged.
So I don't really get the issue you said here. What is the problem exactly ?
@flavio.tarsetti I thought !294 (merged) is already on the staging platform, and now should be available on the staging. Because I checked it earlier and it was working. Now I came back to the staging platform assuming that fix exists and checking the new fix.
The new fix is working but the previous one (regarding the list of experiments at the beginning of the file) is not working as if it never existed. I am confused because in that case your comment in !294 (merged) regarding issues with links resurfacing is irrelevant as well because they are supposed to be fixed in this MR (!288 (merged)).
-
Sorry my bad. I was not clear. Previously on the staging platform we had !294 (merged) . Right now at the moment it's !288 (merged)
As the issue I mentioned was there in the unmerged !294 (merged), it should be here too. Since those bugs are there in both MR, so let's keep this MR opened too so @jdiefenbaugh could fix that.
mentioned in merge request !294 (merged)
added 9 commits
-
29dc178a...155a644b - 6 commits from branch
master - c2a1b7c9 - [reports] smarter access logic for reports' experiments
- 4ae05bdd - [reports] fix nosetests to expect new field
- 67e158e6 - [reports][js] let user know why they cant see exps names
Toggle commit list-
29dc178a...155a644b - 6 commits from branch
@flavio.tarsetti I tested again and it seemed ok to me, however I cannot check the shared experiment situation when I am looking at the report anonymously. I sent you the report directly to you to check.
mentioned in commit f7ed9c32