There are currently two separate issues:
TODO: Update...
Initially, it seems the system was using the following privileges to control access to the edits:
The trouble here is that these privileges are linked to the population resource rather than the phenotype resource itself, which leads to problems fetching and applying the authorisation correctly.
To fix this, I (fredm) propose changing the privileges to:
In conjunction with that, we'll probably need to do the following updates too:
We then need to assign the `resource:metadata:*` privileges at the phenotype/genotype/mRNA resource level, not the InbredSet group level. The `system:systemwide:metadata:*` resource should be assigned at the system level.
The other problem is the naming of the feature here, in both GN2 and GN3.
In GN3, the feature to edit/reject the data is in the `gn3.api.case_attributes` module. When you think about what the code is doing, however, you note that what is changing is the values of the "case attributes" and not actually the "case attributes" themselves -- we are editing data and metadata (values), not the structure of it (case attribute names).
With this in consideration, there are a number of notions to consider/fix as detailed below.
Notion 01: Rather than renaming the privileges as in the **Auth Uncertainty** section above, instead, add new privileges entirely with the names proposed in the sections above. These newly defined privileges can then be grouped into relevant resource-level and system-level roles and assigned appropriately. The code doing the checks can then be refactored as appropriate, renaming `gn3/api/case_attribute.py` to say, `gn3/api/data_metadata.py` or similar.
Notion 02: Leave the `system:**:inbredset:*` privileges for use with editing the "case-attribute" names rather than the data.
Notion 03: Replace the `group:resource:*` privileges with the new `resource:*:*` privileges. We'll need to define the differences between dealing with the resource itself, the attached data in the phenotype/genotype/mRNA traits, and the metadata for said traits -- maybe have:
This will then need to be followed up with updating the code doing the authorisation checks to fix any breakages.
Notion 04: While handling the other notions above, we should also probably separate editing of the metadata from the editing of computationally-significant data.
There appear to be two separate functions handling this that both run when diffs are approved or rejected - __authorised_p__ and __edit_with_authorisation__
I (zsloan) made a commit removing the former (b1c50aed6cefa2a989ef9804f281ff97fec4d42a) and this seems to address an error that was occurring before, but I'm still not aware if there was originally some reason for this.