Repository navigation
feat(rdm.migration): Configurable auto-adding secondary community - #544
Conversation
bf187da to
4eab511
Compare
4eab511 to
53f4ac6
Compare
| """ | ||
| if self.restricted or record.get("access") != "public": | ||
| return False | ||
| if any(file.get("status") for file in entry.get("files", [])): |
There was a problem hiding this comment.
are you sure that this covers everything? I would also check if the output json in access.files has it set to restricted or not
There was a problem hiding this comment.
I tested all the possible scenarios in the test cases. Could you also check that and let me know if something is missing?
There is no access.files populated in this flow. Do you mean here? It only sets enabled key
cds-migrator-kit/cds_migrator_kit/rdm/records/transform/transform.py
Lines 781 to 785 in 059313b
cds-migrator-kit/cds_migrator_kit/rdm/records/transform/transform.py
Lines 240 to 244 in 059313b
There was a problem hiding this comment.
thanks for checking this. One more case comes to mind: we can set the whole collection to restricted, in this case we shouldn't add the scientific community either
There was a problem hiding this comment.
149196f to
7fb15ab
Compare
| return {"owned_by": {"user": owner}} | ||
|
|
||
| def _should_add_scientific_community(self): | ||
| if self.record.restricted or self.record.access_status != "public": |
There was a problem hiding this comment.
I think we could potentially on the record class object have a function which will calculate this if in more reproducible way f.e. has_any_restrictions
| def _should_add_scientific_community(self): | ||
| if self.record.restricted or self.record.access_status != "public": | ||
| return False | ||
| if any(file.get("status") for file in self.dojson_entry.get("files", [])): |
There was a problem hiding this comment.
this I would also "socialize" as access via has_any_restricted_file method of dojson_entry object's class
* Updated to be in sync with the recent codebase refactor
7fb15ab to
0e069b1
Compare
| self.restricted | ||
| or self.access_status != "public" | ||
| or self.has_any_restricted_file() |
There was a problem hiding this comment.
we have 3 things here that say restricted....... can't we use the compute_access output to say it is restricted or not? If the correct value is added in self.restricted then we dont need the other 2 checks....
There was a problem hiding this comment.
I think this is going to be the consolidation part that we need to do with the new dump, compute access I think does not cover it
closes: #542
https://codimd.web.cern.ch/g3bn8n7LRimDeil-TDDIlA