Fix sharing issues where owner project has been previously migrated but not in current migration run - #203
Fix sharing issues where owner project has been previously migrated but not in current migration run#203ruaridhg wants to merge 9 commits into
Conversation
…wner project in specific migration run
…hecks owner project in specific migration run
…- for experiments and assessors
…t appearing in UI - subject is shared w/ data
…t appearing in UI - subject is shared w/ data
ruaridhg
changed the title
p-j-smith
left a comment
p-j-smith
left a comment
There was a problem hiding this comment.
just wondering whether this is an issue you ran into? There are a few parts I'm not sure about (e.g. is setting root.attrib["project"] correct).
I think it might be simpler if we only support migrating all related projects at the same time. Do you know if there's an easy way to check / enforce this?
| sharing_info = self.assessor_sharing.get(assessor.id, {"owner": None, "projects": []}) | ||
| if root.attrib["project"] != self.source_info.id: | ||
| # this project is not the owner of the resource, no need to create it on the destination | ||
| sharing_info["projects"].append(self.destination_info.id) | ||
| owner_project = root.attrib["project"] | ||
| sharing_info["owner"] = owner_project |
There was a problem hiding this comment.
the comment says 'this project is not the owner of the resource' (i.e. root.attrib["project"] is not the owner), but on line 778 root.attrib["project"] is then set as the owner_project
| if ( | ||
| assessor.label | ||
| not in self.destination_connection.projects[self.destination_info.id] | ||
|
|
||
| destination_experiment = ( | ||
| self.destination_connection.projects[self.destination_info.id] | ||
| .subjects[subject.label] | ||
| .experiments[experiment.label] | ||
| .assessors | ||
| ): | ||
| ) |
There was a problem hiding this comment.
just wondering why the check is changing. Does the current check miss some assessors?
I think the first check gets all assessors for the project, and the second only gets the assessors for the experiment. Do you know which is correct?
| # Skip resource creation for this subject if it is shared and this project is not the owner | ||
| sharing_info = self.subject_sharing[subject.label] | ||
| if sharing_info["owner"] != self.destination_info.id: | ||
| continue | ||
| subject_sharing_info = self.subject_sharing[subject.label] | ||
| subject_is_shared = subject_sharing_info["owner"] != self.destination_info.id | ||
|
|
||
| for experiment in subject.experiments: | ||
| self._create_experiment(experiment, destination_datatypes) | ||
| if subject_is_shared: | ||
| continue |
There was a problem hiding this comment.
if the subject is shared, then do we still want to create the experiment? I think we're assuming that if a subject is shared, all resources within that subject should belong to the subject project, not the one it's shared into
| if ( | ||
| owner not in self.destination_connection.projects | ||
| or label not in self.destination_connection.projects[owner].subjects | ||
| ): |
There was a problem hiding this comment.
should the info not always exist in the mapper?
This PR:
migration.pyhandles the likely case where the owner project has been migrated in a previous migration run and the project in the current run has resources shared into it. Previously, the sharing would only work if both the owner project and shared into project were migrated in the same migration run.tests/unit/test_users.pyby moving intotests/integration/test_users.pysince the test uses the actual xnat4test source and destination