Schema updates #9
Conversation
willronchetti
commented
Aug 7, 2023
willronchetti
commented
- Hooks in new schema snovault functionality - see Schema Update 4dn-dcic/snovault#262 for more details
| @@ -23,7 +23,7 @@ ENV NGINX_USER=nginx \ | |||
| NVM_VERSION=v0.39.1 \ | |||
| NODE_VERSION=16.14.0 | |||
|
|
|||
| # Configure Python3.7 venv | |||
| # Configure Python3.9 venv | |||
There was a problem hiding this comment.
Maybe just remove the version here so it's all in one place? It's just not necessary to say what version in this particular comment.
| @@ -8,7 +8,7 @@ def login(self, context, request, *, samesite): | |||
|
|
|||
| def namespaced_authentication_policy_authenticated_userid(self, namespaced_authentication_policy, request, | |||
| set_user_info_property): | |||
| set_user_info_property = False | |||
| set_user_info_property = True | |||
There was a problem hiding this comment.
I still haven't figured out what this does. (i.e., I think a comment wouldn't hurt)
| for key in ['access_key_id', 'file_format', 'title']: | ||
| assert key in props | ||
| # check access key id | ||
| assert props['access_key_id']['comment'] == 'Only admins are allowed to set this value.' |
There was a problem hiding this comment.
Is comment really the name we define in our own data model? Probably too late(?), but I so wish it said another word (like "restriction_description" so it doesn't look meta-syntactic (and hence invisible to computation).
Unrelated, could the string you're looking for be a named constant so that light edits of the text don't break the tests? Or is the place it exists elsewhere only in the schema's .json file? Since I think it's testing the merge, I would accept as a compromise that the string is the same as the string in the thing it's merging from... In fact, I'm a little surprised that you're not just testing that the tree matches that other tree. At a previous employer, I had a testing routine (and I think unittest has a similar one built into it as a .assertxxx operation) that checks that a given dictionary has at least the parts given by another given dictionary (ignoring additional other things that might have been inserted).
| """ Tests admins can do various actions, including purging items """ | ||
|
|
||
| @staticmethod | ||
| def test_admin_can_purge(testapp, fastq_format): |
There was a problem hiding this comment.
Should there not also be a test_non_admin_cant_purge?
