From 0046281639ba7321312d1827bc77f29ba7debca5 Mon Sep 17 00:00:00 2001 From: Jon Van Oast Date: Mon, 8 Jan 2024 12:34:04 -0700 Subject: [PATCH 01/18] WIP --- app/modules/collaborations/models.py | 43 ++++++++- app/modules/notifications/models.py | 137 +++++++++++++++++++++++++++ 2 files changed, 179 insertions(+), 1 deletion(-) diff --git a/app/modules/collaborations/models.py b/app/modules/collaborations/models.py index 5afc75877..1e5ee7f75 100644 --- a/app/modules/collaborations/models.py +++ b/app/modules/collaborations/models.py @@ -54,6 +54,9 @@ class CollaborationUserAssociations(db.Model, HoustonModel): read_approval_state = db.Column( db.String(length=32), default=CollaborationUserState.PENDING, nullable=False ) + export_approval_state = db.Column( + db.String(length=32), default=CollaborationUserState.NOT_INITIATED, nullable=False + ) edit_approval_state = db.Column( db.String(length=32), default=CollaborationUserState.NOT_INITIATED, nullable=False ) @@ -64,6 +67,9 @@ def delete(self): def has_read(self): return self.collaboration.user_has_read_access(self.user_guid) + def has_export(self): + return self.collaboration.user_has_export_access(self.user_guid) + def has_edit(self): return self.collaboration.user_has_edit_access(self.user_guid) @@ -93,6 +99,12 @@ class Collaboration(db.Model, HoustonModel): init_req_notification_guid = db.Column( db.GUID, db.ForeignKey('notification.guid'), nullable=True ) + export_initiator_guid = db.Column( + db.GUID, db.ForeignKey('user.guid'), index=True, nullable=True + ) + export_req_notification_guid = db.Column( + db.GUID, db.ForeignKey('notification.guid'), nullable=True + ) edit_initiator_guid = db.Column( db.GUID, db.ForeignKey('user.guid'), index=True, nullable=True ) @@ -123,6 +135,7 @@ def __init__(self, members, initiator_user, notify_users=True, **kwargs): ) self.initiator_guid = None if manager_created else initiator_user.guid + self.export_initiator_guid = None self.edit_initiator_guid = None for user in members: @@ -133,7 +146,8 @@ def __init__(self, members, initiator_user, notify_users=True, **kwargs): collaboration=self, user=user ) - # Edit not enabled on creation + # Export and edit not enabled on creation + collab_user_assoc.export_approval_state = CollaborationUserState.NOT_INITIATED collab_user_assoc.edit_approval_state = CollaborationUserState.NOT_INITIATED with db.session.begin(subtransactions=True): db.session.add(collab_user_assoc) @@ -166,6 +180,12 @@ def get_users_for_read(cls, user): user, CollaborationUserAssociations.read_approval_state ) + @classmethod + def get_users_for_export(cls, user): + return cls.get_users_for_approval_state( + user, CollaborationUserAssociations.export_approval_state + ) + @classmethod def get_users_for_write(cls, user): return cls.get_users_for_approval_state( @@ -226,6 +246,8 @@ def notify_pending_users(self): if ( collab_user_assoc.read_approval_state == CollaborationUserState.PENDING + or collab_user_assoc.export_approval_state + == CollaborationUserState.PENDING or collab_user_assoc.edit_approval_state == CollaborationUserState.PENDING ): from app.modules.notifications.models import NotificationType @@ -244,6 +266,16 @@ def notify_pending_users(self): NotificationType.collab_request, ) + if ( + collab_user_assoc.export_approval_state + == CollaborationUserState.PENDING + ): + self._notify_user( + other_user_assoc.user, + collab_user_assoc.user, + NotificationType.collab_export_request, + ) + if ( collab_user_assoc.edit_approval_state == CollaborationUserState.PENDING @@ -267,6 +299,8 @@ def _notify_user(self, sending_user, receiving_user, notification_type): if notification_type is NotificationType.collab_request: self.init_req_notification_guid = notif.guid + elif notification_type is NotificationType.collab_export_request: + self.export_req_notification_guid = notif.guid elif notification_type is NotificationType.collab_edit_request: self.edit_req_notification_guid = notif.guid @@ -274,6 +308,8 @@ def _notify_user(self, sending_user, receiving_user, notification_type): fully_resolved_notification_states = { NotificationType.collab_edit_approved, NotificationType.collab_edit_revoke, + NotificationType.collab_export_approved, + NotificationType.collab_export_revoke, NotificationType.collab_revoke, NotificationType.collab_manager_revoke, NotificationType.collab_denied, @@ -286,6 +322,8 @@ def _notify_user(self, sending_user, receiving_user, notification_type): elif notification_type in fully_resolved_notification_states: if self.init_req_notification_guid: Notification.resolve(self.init_req_notification_guid) + if self.export_req_notification_guid: + Notification.resolve(self.export_req_notification_guid) if self.edit_req_notification_guid: Notification.resolve(self.edit_req_notification_guid) @@ -296,11 +334,13 @@ def get_user_data_as_json(self): for association in self.collaboration_user_associations: assoc_data = BaseUserSchema().dump(association.user).data assoc_data['viewState'] = association.read_approval_state + assoc_data['exportState'] = association.export_approval_state assoc_data['editState'] = association.edit_approval_state user_data[str(association.user.guid)] = assoc_data return user_data + # FIXME stopped here def _is_state_transition_valid(self, association, new_state, is_edit=False): ret_val = False old_state = ( @@ -395,6 +435,7 @@ def _send_notification_for_change(self, user_guid, association, state, is_edit): notif_type, ) + # def set_approval_state_for_user(self, user_guid, state, collab_type='view'): def set_approval_state_for_user(self, user_guid, state, is_edit=False): assert user_guid success = False diff --git a/app/modules/notifications/models.py b/app/modules/notifications/models.py index 8d5866a55..8674f44a3 100644 --- a/app/modules/notifications/models.py +++ b/app/modules/notifications/models.py @@ -25,6 +25,14 @@ class NotificationType(str, enum.Enum): # other user approved collaboration request collab_approved = 'collaboration_approved' collab_denied = 'collaboration_denied' + # other user requests export collaboration with you + collab_export_request = 'collaboration_export_request' + # other user approved export request + collab_export_approved = 'collaboration_export_approved' + # other user denied export request + collab_export_denied = 'collaboration_export_denied' + # other user revokes the existing export part of your collaboration + collab_export_revoke = 'collaboration_export_revoke' # other user requests edit collaboration with you collab_edit_request = 'collaboration_edit_request' # other user approved edit request @@ -39,6 +47,12 @@ class NotificationType(str, enum.Enum): # A user manager has denied a collaboration for you with another user collab_manager_denied = 'collaboration_manager_denied' # A user manager has revoked a collaboration for you with another user + # user_manager approved export request + collab_manager_export_approved = 'collaboration_manager_export_approved' + # user_manager denied export request + collab_manager_export_denied = 'collaboration_manager_export_denied' + # user_manager revokes the existing export part of your collaboration + collab_manager_export_revoke = 'collaboration_manager_export_revoke' # user_manager approved edit request collab_manager_edit_approved = 'collaboration_manager_edit_approved' # user_manager denied edit request @@ -76,6 +90,22 @@ class NotificationChannel(str, enum.Enum): NotificationChannel.rest: True, NotificationChannel.email: False, }, + NotificationType.collab_export_request: { + NotificationChannel.rest: True, + NotificationChannel.email: False, + }, + NotificationType.collab_export_approved: { + NotificationChannel.rest: True, + NotificationChannel.email: False, + }, + NotificationType.collab_export_revoke: { + NotificationChannel.rest: True, + NotificationChannel.email: False, + }, + NotificationType.collab_export_denied: { + NotificationChannel.rest: True, + NotificationChannel.email: False, + }, NotificationType.collab_edit_request: { NotificationChannel.rest: True, NotificationChannel.email: False, @@ -108,6 +138,18 @@ class NotificationChannel(str, enum.Enum): NotificationChannel.rest: True, NotificationChannel.email: False, }, + NotificationType.collab_manager_export_approved: { + NotificationChannel.rest: True, + NotificationChannel.email: False, + }, + NotificationType.collab_manager_export_revoke: { + NotificationChannel.rest: True, + NotificationChannel.email: False, + }, + NotificationType.collab_manager_export_denied: { + NotificationChannel.rest: True, + NotificationChannel.email: False, + }, NotificationType.collab_manager_edit_approved: { NotificationChannel.rest: True, NotificationChannel.email: False, @@ -178,6 +220,53 @@ class NotificationChannel(str, enum.Enum): }, 'resolve_on_read': True, }, + NotificationType.collab_export_request: { + 'email_template_name': 'collaboration_export_request', + 'email_digest_content_template': 'collaboration_export_request_digest', + 'mandatory_fields': { + 'collaboration_guid', + 'user1_name', + 'user2_name', + 'user1_guid', + 'user2_guid', + }, + }, + NotificationType.collab_export_approved: { + 'email_template_name': 'collaboration_export_approved', + 'email_digest_content_template': 'collaboration_export_approved_digest', + 'mandatory_fields': { + 'collaboration_guid', + 'user1_name', + 'user2_name', + 'user1_guid', + 'user2_guid', + }, + 'resolve_on_read': True, + }, + NotificationType.collab_export_denied: { + 'email_template_name': 'collaboration_export_denied', + 'email_digest_content_template': 'collaboration_export_denied_digest', + 'mandatory_fields': { + 'collaboration_guid', + 'user1_name', + 'user2_name', + 'user1_guid', + 'user2_guid', + }, + 'resolve_on_read': True, + }, + NotificationType.collab_export_revoke: { + 'email_template_name': 'collaboration_export_revoke', + 'email_digest_content_template': 'collaboration_export_revoke_digest', + 'mandatory_fields': { + 'collaboration_guid', + 'user1_name', + 'user2_name', + 'user1_guid', + 'user2_guid', + }, + 'resolve_on_read': True, + }, NotificationType.collab_edit_request: { 'email_template_name': 'collaboration_edit_request', 'email_digest_content_template': 'collaboration_edit_request_digest', @@ -285,6 +374,54 @@ class NotificationChannel(str, enum.Enum): 'resolve_on_read': True, 'managed': True, }, + NotificationType.collab_manager_export_approved: { + 'email_template_name': 'collaboration_manger_export_approved', + 'email_digest_content_template': 'collaboration_manager_export_approved_digest', + 'mandatory_fields': { + 'collaboration_guid', + 'user1_name', + 'user2_name', + 'user1_guid', + 'user2_guid', + 'manager_name', + 'manager_guid', + }, + 'allow_multiple': True, + 'resolve_on_read': True, + 'managed': True, + }, + NotificationType.collab_manager_export_denied: { + 'email_template_name': 'collaboration_manger_export_denied', + 'email_digest_content_template': 'collaboration_manager_export_denied_digest', + 'mandatory_fields': { + 'collaboration_guid', + 'user1_name', + 'user2_name', + 'user1_guid', + 'user2_guid', + 'manager_name', + 'manager_guid', + }, + 'allow_multiple': True, + 'resolve_on_read': True, + 'managed': True, + }, + NotificationType.collab_manager_export_revoke: { + 'email_template_name': 'collaboration_manger_export_revoke', + 'email_digest_content_template': 'collaboration_manager_export_revoke_digest', + 'mandatory_fields': { + 'collaboration_guid', + 'user1_name', + 'user2_name', + 'user1_guid', + 'user2_guid', + 'manager_name', + 'manager_guid', + }, + 'allow_multiple': True, + 'resolve_on_read': True, + 'managed': True, + }, NotificationType.collab_manager_edit_approved: { 'email_template_name': 'collaboration_manger_edit_approved', 'email_digest_content_template': 'collaboration_manager_edit_approved_digest', From 5e5b0c337a012c51f71f85648fba2bc697b4f761 Mon Sep 17 00:00:00 2001 From: Jon Van Oast Date: Mon, 8 Jan 2024 17:40:25 -0700 Subject: [PATCH 02/18] baby steps --- app/modules/collaborations/models.py | 51 ++++++++++----------- app/modules/collaborations/parameters.py | 6 +-- tests/modules/collaborations/test_models.py | 2 +- 3 files changed, 28 insertions(+), 31 deletions(-) diff --git a/app/modules/collaborations/models.py b/app/modules/collaborations/models.py index 1e5ee7f75..cc6635482 100644 --- a/app/modules/collaborations/models.py +++ b/app/modules/collaborations/models.py @@ -340,12 +340,11 @@ def get_user_data_as_json(self): return user_data - # FIXME stopped here - def _is_state_transition_valid(self, association, new_state, is_edit=False): + def _is_state_transition_valid(self, association, new_state, level='view'): ret_val = False old_state = ( association.edit_approval_state - if is_edit + if level == 'edit' else association.read_approval_state ) # Only certain transitions are permitted @@ -372,26 +371,26 @@ def _is_state_transition_valid(self, association, new_state, is_edit=False): return ret_val - def _send_notification_for_manager_change(self, association, state, is_edit): + def _send_notification_for_manager_change(self, association, state, level): from app.modules.notifications.models import NotificationType notif_type = None if state == CollaborationUserState.REVOKED: notif_type = ( NotificationType.collab_manager_edit_revoke - if is_edit + if level == 'edit' else NotificationType.collab_manager_revoke ) elif state == CollaborationUserState.APPROVED: notif_type = ( NotificationType.collab_manager_edit_approved - if is_edit + if level == 'edit' else NotificationType.collab_manager_create ) elif state == CollaborationUserState.DENIED: notif_type = ( NotificationType.collab_manager_edit_denied - if is_edit + if level == 'edit' else NotificationType.collab_manager_denied ) @@ -404,26 +403,26 @@ def _send_notification_for_manager_change(self, association, state, is_edit): notif_type, ) - def _send_notification_for_change(self, user_guid, association, state, is_edit): + def _send_notification_for_change(self, user_guid, association, state, level): from app.modules.notifications.models import NotificationType notif_type = None if state == CollaborationUserState.REVOKED: notif_type = ( NotificationType.collab_edit_revoke - if is_edit + if level == 'edit' else NotificationType.collab_revoke ) elif state == CollaborationUserState.APPROVED: notif_type = ( NotificationType.collab_edit_approved - if is_edit + if level == 'edit' else NotificationType.collab_approved ) elif state == CollaborationUserState.DENIED: notif_type = ( NotificationType.collab_edit_denied - if is_edit + if level == 'edit' else NotificationType.collab_denied ) @@ -436,7 +435,7 @@ def _send_notification_for_change(self, user_guid, association, state, is_edit): ) # def set_approval_state_for_user(self, user_guid, state, collab_type='view'): - def set_approval_state_for_user(self, user_guid, state, is_edit=False): + def set_approval_state_for_user(self, user_guid, state, level='view'): assert user_guid success = False if state not in CollaborationUserState.ALLOWED_STATES: @@ -448,12 +447,12 @@ def set_approval_state_for_user(self, user_guid, state, is_edit=False): for association in self.collaboration_user_associations: if association.user_guid != user_guid: continue - if is_edit: - if self._is_state_transition_valid(association, state, True): + if level == 'edit': + if self._is_state_transition_valid(association, state, 'edit'): association.edit_approval_state = state success = True else: - if self._is_state_transition_valid(association, state, False): + if self._is_state_transition_valid(association, state, 'view'): association.read_approval_state = state # If a user revokes view and previously allowed edit, they automatically @@ -470,25 +469,23 @@ def set_approval_state_for_user(self, user_guid, state, is_edit=False): with db.session.begin(subtransactions=True): db.session.merge(association) if current_user and current_user.guid != user_guid: - self._send_notification_for_manager_change( - association, state, is_edit - ) + self._send_notification_for_manager_change(association, state, level) else: self._send_notification_for_change( - user_guid, association, state, is_edit + user_guid, association, state, level ) break return success - def set_approval_state_for_all(self, state, is_edit=False): + def set_approval_state_for_all(self, state, level='view'): if state not in CollaborationUserState.ALLOWED_STATES: raise ValueError( f'State "{state}" not in allowed states: {", ".join(CollaborationUserState.ALLOWED_STATES)}' ) # Don't transition any user state unless all transitions are valid for association in self.collaboration_user_associations: - if not self._is_state_transition_valid(association, state, is_edit): + if not self._is_state_transition_valid(association, state, level): raise HoustonException( log, f'Not permitted to move user {association.user_guid} to state {state}', @@ -496,14 +493,14 @@ def set_approval_state_for_all(self, state, is_edit=False): changed = False for association in self.collaboration_user_associations: - if is_edit: + if level == 'edit': if association.edit_approval_state != state: changed = True association.edit_approval_state = state # If a user manager approves edit make sure view is not left inconsistent if ( state == CollaborationUserState.APPROVED - and self._is_state_transition_valid(association, state, False) + and self._is_state_transition_valid(association, state, 'view') ): association.read_approval_state = state else: @@ -514,7 +511,7 @@ def set_approval_state_for_all(self, state, is_edit=False): # If a user manager revokes view make sure edit is not left inconsistent if ( state == CollaborationUserState.REVOKED - and self._is_state_transition_valid(association, state, True) + and self._is_state_transition_valid(association, state, 'edit') ): association.edit_approval_state = state @@ -525,7 +522,7 @@ def set_approval_state_for_all(self, state, is_edit=False): # If something changed both users get a notification, it doesn't matter which association we send so # just pick one self._send_notification_for_manager_change( - self.collaboration_user_associations[0], state, is_edit + self.collaboration_user_associations[0], state, level ) return True @@ -574,13 +571,13 @@ def initiate_edit_with_other_user(self): and other_assoc.read_approval_state == CollaborationUserState.APPROVED ): if self._is_state_transition_valid( - my_assoc, CollaborationUserState.APPROVED, is_edit=True + my_assoc, CollaborationUserState.APPROVED, level='edit' ): my_assoc.edit_approval_state = CollaborationUserState.APPROVED with db.session.begin(subtransactions=True): db.session.merge(my_assoc) if self._is_state_transition_valid( - other_assoc, CollaborationUserState.PENDING, is_edit=True + other_assoc, CollaborationUserState.PENDING, level='edit' ): other_assoc.edit_approval_state = CollaborationUserState.PENDING with db.session.begin(subtransactions=True): diff --git a/app/modules/collaborations/parameters.py b/app/modules/collaborations/parameters.py index 457cce4ce..4c86d977d 100644 --- a/app/modules/collaborations/parameters.py +++ b/app/modules/collaborations/parameters.py @@ -86,7 +86,7 @@ def replace(cls, obj, field, value, state): elif field == 'edit_permission': if rules.ObjectActionRule(obj, AccessOperation.WRITE).check(): ret_val = obj.set_approval_state_for_user( - current_user.guid, value, is_edit=True + current_user.guid, value, level='edit' ) elif field == 'managed_view_permission': @@ -102,9 +102,9 @@ def replace(cls, obj, field, value, state): user_guid, permission = cls.get_managed_values(field, value) if user_guid: ret_val = obj.set_approval_state_for_user( - user_guid, permission, is_edit=True + user_guid, permission, level='edit' ) else: - ret_val = obj.set_approval_state_for_all(permission, is_edit=True) + ret_val = obj.set_approval_state_for_all(permission, level='edit') return ret_val diff --git a/tests/modules/collaborations/test_models.py b/tests/modules/collaborations/test_models.py index ddc2d141a..f25fff8c9 100644 --- a/tests/modules/collaborations/test_models.py +++ b/tests/modules/collaborations/test_models.py @@ -132,7 +132,7 @@ def test_collaboration_edit_state_changes(db, collab_user_a, collab_user_b, requ if association.user_guid == collab_user_b.guid: assert association.edit_approval_state == CollaborationUserState.PENDING collab.set_approval_state_for_user( - collab_user_b.guid, CollaborationUserState.APPROVED, is_edit=True + collab_user_b.guid, CollaborationUserState.APPROVED, level='edit' ) for association in collab.collaboration_user_associations: From d3267de81dea73b5530b5ee9254f1bef5d254fd7 Mon Sep 17 00:00:00 2001 From: Jon Van Oast Date: Mon, 8 Jan 2024 17:42:44 -0700 Subject: [PATCH 03/18] db migration (we hope) --- migrations/versions/effd65fb089e_.py | 82 ++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 migrations/versions/effd65fb089e_.py diff --git a/migrations/versions/effd65fb089e_.py b/migrations/versions/effd65fb089e_.py new file mode 100644 index 000000000..53afff97f --- /dev/null +++ b/migrations/versions/effd65fb089e_.py @@ -0,0 +1,82 @@ +# -*- coding: utf-8 -*- +"""empty message + +Revision ID: effd65fb089e +Revises: 53b0fec87272 +Create Date: 2024-01-05 18:24:21.360901 + +""" +import sqlalchemy as sa +from alembic import op + +import app +import app.extensions + +# revision identifiers, used by Alembic. +revision = 'effd65fb089e' +down_revision = '53b0fec87272' + + +def upgrade(): + """ + Upgrade Semantic Description: + ENTER DESCRIPTION HERE + """ + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table('collaboration', schema=None) as batch_op: + batch_op.add_column( + sa.Column('export_initiator_guid', app.extensions.GUID(), nullable=True) + ) + batch_op.add_column( + sa.Column( + 'export_req_notification_guid', app.extensions.GUID(), nullable=True + ) + ) + batch_op.create_index( + batch_op.f('ix_collaboration_export_initiator_guid'), + ['export_initiator_guid'], + unique=False, + ) + batch_op.create_foreign_key( + batch_op.f('fk_collaboration_export_initiator_guid_user'), + 'user', + ['export_initiator_guid'], + ['guid'], + ) + batch_op.create_foreign_key( + batch_op.f('fk_collaboration_export_req_notification_guid_notification'), + 'notification', + ['export_req_notification_guid'], + ['guid'], + ) + + with op.batch_alter_table('collaboration_user_associations', schema=None) as batch_op: + batch_op.add_column( + sa.Column('export_approval_state', sa.String(length=32), nullable=False) + ) + + # ### end Alembic commands ### + + +def downgrade(): + """ + Downgrade Semantic Description: + ENTER DESCRIPTION HERE + """ + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table('collaboration_user_associations', schema=None) as batch_op: + batch_op.drop_column('export_approval_state') + + with op.batch_alter_table('collaboration', schema=None) as batch_op: + batch_op.drop_constraint( + batch_op.f('fk_collaboration_export_req_notification_guid_notification'), + type_='foreignkey', + ) + batch_op.drop_constraint( + batch_op.f('fk_collaboration_export_initiator_guid_user'), type_='foreignkey' + ) + batch_op.drop_index(batch_op.f('ix_collaboration_export_initiator_guid')) + batch_op.drop_column('export_req_notification_guid') + batch_op.drop_column('export_initiator_guid') + + # ### end Alembic commands ### From e230a1109bc0e34154134a11d01a402d4c7a0d30 Mon Sep 17 00:00:00 2001 From: Jon Van Oast Date: Wed, 10 Jan 2024 16:04:13 -0700 Subject: [PATCH 04/18] handling 3 levels, part 1 --- app/modules/collaborations/models.py | 86 +++++++++++++++++----------- 1 file changed, 51 insertions(+), 35 deletions(-) diff --git a/app/modules/collaborations/models.py b/app/modules/collaborations/models.py index cc6635482..0a97ba677 100644 --- a/app/modules/collaborations/models.py +++ b/app/modules/collaborations/models.py @@ -312,6 +312,7 @@ def _notify_user(self, sending_user, receiving_user, notification_type): NotificationType.collab_export_revoke, NotificationType.collab_revoke, NotificationType.collab_manager_revoke, + NotificationType.collab_manager_export_revoke, NotificationType.collab_denied, } @@ -342,11 +343,16 @@ def get_user_data_as_json(self): def _is_state_transition_valid(self, association, new_state, level='view'): ret_val = False - old_state = ( - association.edit_approval_state - if level == 'edit' - else association.read_approval_state - ) + if level == 'view': + old_state = association.read_approval_state + elif level == 'export': + old_state = association.export_approval_state + elif level == 'edit': + old_state = association.edit_approval_state + else: + log.warning(f'invalid level "{level}"') + return False + # Only certain transitions are permitted if old_state == CollaborationUserState.NOT_INITIATED: ret_val = new_state in [ @@ -376,23 +382,28 @@ def _send_notification_for_manager_change(self, association, state, level): notif_type = None if state == CollaborationUserState.REVOKED: - notif_type = ( - NotificationType.collab_manager_edit_revoke - if level == 'edit' - else NotificationType.collab_manager_revoke - ) + if level == 'view': + notif_type = NotificationType.collab_manager_revoke + elif level == 'export': + notif_type = NotificationType.collab_manager_export_revoke + elif level == 'edit': + notif_type = NotificationType.collab_manager_edit_revoke + elif state == CollaborationUserState.APPROVED: - notif_type = ( - NotificationType.collab_manager_edit_approved - if level == 'edit' - else NotificationType.collab_manager_create - ) + if level == 'view': + notif_type = NotificationType.collab_manager_create + elif level == 'export': + notif_type = NotificationType.collab_manager_export_approved + elif level == 'edit': + notif_type = NotificationType.collab_manager_edit_approved + elif state == CollaborationUserState.DENIED: - notif_type = ( - NotificationType.collab_manager_edit_denied - if level == 'edit' - else NotificationType.collab_manager_denied - ) + if level == 'view': + notif_type = NotificationType.collab_manager_denied + elif level == 'export': + notif_type = NotificationType.collab_manager_export_denied + elif level == 'edit': + notif_type = NotificationType.collab_manager_edit_denied if notif_type: # Inform the user that the manager has changed their permission @@ -408,23 +419,28 @@ def _send_notification_for_change(self, user_guid, association, state, level): notif_type = None if state == CollaborationUserState.REVOKED: - notif_type = ( - NotificationType.collab_edit_revoke - if level == 'edit' - else NotificationType.collab_revoke - ) + if level == 'view': + notif_type = NotificationType.collab_revoke + elif level == 'export': + notif_type = NotificationType.collab_export_revoke + elif level == 'edit': + notif_type = NotificationType.collab_edit_revoke + elif state == CollaborationUserState.APPROVED: - notif_type = ( - NotificationType.collab_edit_approved - if level == 'edit' - else NotificationType.collab_approved - ) + if level == 'view': + notif_type = NotificationType.collab_approved + elif level == 'export': + notif_type = NotificationType.collab_export_approved + elif level == 'edit': + notif_type = NotificationType.collab_edit_approved + elif state == CollaborationUserState.DENIED: - notif_type = ( - NotificationType.collab_edit_denied - if level == 'edit' - else NotificationType.collab_denied - ) + if level == 'view': + notif_type = NotificationType.collab_denied + elif level == 'export': + notif_type = NotificationType.collab_export_denied + elif level == 'edit': + notif_type = NotificationType.collab_edit_denied if notif_type: # inform the other user that their collaborator changed the permission From e01e2e617b9cc3e58b47dcdcca9bb5677a68401c Mon Sep 17 00:00:00 2001 From: Jon Van Oast Date: Thu, 11 Jan 2024 14:53:44 -0700 Subject: [PATCH 05/18] more level handling --- app/modules/collaborations/models.py | 49 +++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/app/modules/collaborations/models.py b/app/modules/collaborations/models.py index 0a97ba677..e1af0515c 100644 --- a/app/modules/collaborations/models.py +++ b/app/modules/collaborations/models.py @@ -467,6 +467,18 @@ def set_approval_state_for_user(self, user_guid, state, level='view'): if self._is_state_transition_valid(association, state, 'edit'): association.edit_approval_state = state success = True + elif level == 'export': + if self._is_state_transition_valid(association, state, 'view'): + association.read_approval_state = state + # If a user revokes export and previously allowed edit, they automatically + # revoke edit too + if ( + state == CollaborationUserState.REVOKED + and association.edit_approval_state + == CollaborationUserState.APPROVED + ): + association.edit_approval_state = state + success = True else: if self._is_state_transition_valid(association, state, 'view'): association.read_approval_state = state @@ -479,6 +491,13 @@ def set_approval_state_for_user(self, user_guid, state, level='view'): == CollaborationUserState.APPROVED ): association.edit_approval_state = state + # ditto for export + if ( + state == CollaborationUserState.REVOKED + and association.export_approval_state + == CollaborationUserState.APPROVED + ): + association.export_approval_state = state success = True if success: @@ -513,23 +532,51 @@ def set_approval_state_for_all(self, state, level='view'): if association.edit_approval_state != state: changed = True association.edit_approval_state = state + # If a user manager approves edit make sure view and export are not left inconsistent + if ( + state == CollaborationUserState.APPROVED + and self._is_state_transition_valid(association, state, 'view') + ): + association.read_approval_state = state + if ( + state == CollaborationUserState.APPROVED + and self._is_state_transition_valid(association, state, 'export') + ): + association.export_approval_state = state + + elif level == 'export': + if association.export_approval_state != state: + changed = True + association.export_approval_state = state # If a user manager approves edit make sure view is not left inconsistent if ( state == CollaborationUserState.APPROVED and self._is_state_transition_valid(association, state, 'view') ): association.read_approval_state = state + # If a user manager revokes export make sure edit is not left inconsistent + if ( + state == CollaborationUserState.REVOKED + and self._is_state_transition_valid(association, state, 'edit') + ): + association.edit_approval_state = state + else: if association.read_approval_state != state: association.read_approval_state = state changed = True - # If a user manager revokes view make sure edit is not left inconsistent + # If a user manager revokes view make sure edit and export are not left inconsistent if ( state == CollaborationUserState.REVOKED and self._is_state_transition_valid(association, state, 'edit') ): association.edit_approval_state = state + if ( + state == CollaborationUserState.REVOKED + and self._is_state_transition_valid(association, state, 'export') + ): + association.export_approval_state = state with db.session.begin(subtransactions=True): db.session.merge(association) From c546a6814d57bc77b767456f57c4f920eb39f029 Mon Sep 17 00:00:00 2001 From: Jon Van Oast Date: Tue, 16 Jan 2024 15:09:39 -0700 Subject: [PATCH 06/18] export-request to mirror edit-request --- app/modules/collaborations/models.py | 29 +++++++++++++++++++++++-- app/modules/collaborations/resources.py | 25 +++++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/app/modules/collaborations/models.py b/app/modules/collaborations/models.py index e1af0515c..b757895a3 100644 --- a/app/modules/collaborations/models.py +++ b/app/modules/collaborations/models.py @@ -625,13 +625,38 @@ def get_other_user(self, user_guid): return other_assoc.user return None - def initiate_edit_with_other_user(self): - self.edit_initiator_guid = current_user.guid + def initiate_export_with_other_user(self): + self.export_initiator_guid = current_user.guid my_assoc = self._get_association_for_user(current_user.guid) other_assoc = self._get_association_for_other_user(current_user.guid) if ( my_assoc.read_approval_state == CollaborationUserState.APPROVED and other_assoc.read_approval_state == CollaborationUserState.APPROVED + ): + if self._is_state_transition_valid( + my_assoc, CollaborationUserState.APPROVED, level='export' + ): + my_assoc.export_approval_state = CollaborationUserState.APPROVED + with db.session.begin(subtransactions=True): + db.session.merge(my_assoc) + if self._is_state_transition_valid( + other_assoc, CollaborationUserState.PENDING, level='export' + ): + other_assoc.export_approval_state = CollaborationUserState.PENDING + with db.session.begin(subtransactions=True): + db.session.merge(other_assoc) + else: + raise HoustonException( + log, 'Unable to start export on unapproved collaboration', obj=self + ) + + def initiate_edit_with_other_user(self): + self.edit_initiator_guid = current_user.guid + my_assoc = self._get_association_for_user(current_user.guid) + other_assoc = self._get_association_for_other_user(current_user.guid) + if ( + my_assoc.export_approval_state == CollaborationUserState.APPROVED + and other_assoc.export_approval_state == CollaborationUserState.APPROVED ): if self._is_state_transition_valid( my_assoc, CollaborationUserState.APPROVED, level='edit' diff --git a/app/modules/collaborations/resources.py b/app/modules/collaborations/resources.py index 51b5240c0..04fd49e0f 100644 --- a/app/modules/collaborations/resources.py +++ b/app/modules/collaborations/resources.py @@ -256,6 +256,31 @@ def delete(self, collaboration): return None +@api.route('/export_request/') +@api.login_required(oauth_scopes=['collaborations:write']) +@api.response( + code=HTTPStatus.NOT_FOUND, + description='Collaboration not found.', +) +@api.resolve_object_by_model(Collaboration, 'collaboration') +class CollaborationExportRequest(Resource): + """ + Request that a specific collaboration is escalated to export + """ + + @api.response(schemas.DetailedCollaborationSchema()) + @api.response(code=HTTPStatus.CONFLICT) + def post(self, collaboration): + + try: + collaboration.initiate_export_with_other_user() + except HoustonException as ex: + abort(ex.status_code, ex.message) + collaboration.notify_pending_users() + + return collaboration + + @api.route('/edit_request/') @api.login_required(oauth_scopes=['collaborations:write']) @api.response( From fba7ade16dbc752b6e1b18beb3646f6c1e39ee53 Mon Sep 17 00:00:00 2001 From: Jon Van Oast Date: Tue, 16 Jan 2024 15:54:47 -0700 Subject: [PATCH 07/18] "bug fixes and improvements" --- app/modules/collaborations/models.py | 17 +++++++++++++++-- app/modules/collaborations/parameters.py | 18 ++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/app/modules/collaborations/models.py b/app/modules/collaborations/models.py index b757895a3..23f00f4f5 100644 --- a/app/modules/collaborations/models.py +++ b/app/modules/collaborations/models.py @@ -450,7 +450,6 @@ def _send_notification_for_change(self, user_guid, association, state, level): notif_type, ) - # def set_approval_state_for_user(self, user_guid, state, collab_type='view'): def set_approval_state_for_user(self, user_guid, state, level='view'): assert user_guid success = False @@ -469,7 +468,7 @@ def set_approval_state_for_user(self, user_guid, state, level='view'): success = True elif level == 'export': if self._is_state_transition_valid(association, state, 'view'): - association.read_approval_state = state + association.export_approval_state = state # If a user revokes export and previously allowed edit, they automatically # revoke edit too if ( @@ -605,6 +604,20 @@ def user_has_read_access(self, user_guid): return ret_val + def user_has_export_access(self, user_guid): + ret_val = False + assert isinstance(user_guid, uuid.UUID) + + my_assoc = self._get_association_for_user(user_guid) + other_assoc = self._get_association_for_other_user(user_guid) + + if my_assoc and other_assoc: + ret_val = ( + my_assoc.export_approval_state == CollaborationUserState.APPROVED + and other_assoc.export_approval_state == CollaborationUserState.APPROVED + ) + return ret_val + def user_has_edit_access(self, user_guid): ret_val = False assert isinstance(user_guid, uuid.UUID) diff --git a/app/modules/collaborations/parameters.py b/app/modules/collaborations/parameters.py index 4c86d977d..c75655b49 100644 --- a/app/modules/collaborations/parameters.py +++ b/app/modules/collaborations/parameters.py @@ -43,8 +43,10 @@ class PatchCollaborationDetailsParameters(PatchJSONParameters): PATH_CHOICES = ( '/view_permission', '/edit_permission', + '/export_permission', '/managed_view_permission', '/managed_edit_permission', + '/managed_export_permission', ) @classmethod @@ -83,6 +85,12 @@ def replace(cls, obj, field, value, state): if rules.ObjectActionRule(obj, AccessOperation.WRITE).check(): ret_val = obj.set_approval_state_for_user(current_user.guid, value) + elif field == 'export_permission': + if rules.ObjectActionRule(obj, AccessOperation.WRITE).check(): + ret_val = obj.set_approval_state_for_user( + current_user.guid, value, level='export' + ) + elif field == 'edit_permission': if rules.ObjectActionRule(obj, AccessOperation.WRITE).check(): ret_val = obj.set_approval_state_for_user( @@ -97,6 +105,16 @@ def replace(cls, obj, field, value, state): else: ret_val = obj.set_approval_state_for_all(permission) + elif field == 'managed_export_permission': + if current_user.is_user_manager: + user_guid, permission = cls.get_managed_values(field, value) + if user_guid: + ret_val = obj.set_approval_state_for_user( + user_guid, permission, level='export' + ) + else: + ret_val = obj.set_approval_state_for_all(permission, level='export') + elif field == 'managed_edit_permission': if current_user.is_user_manager: user_guid, permission = cls.get_managed_values(field, value) From 15acceae2f0f66778d3b070f450310fa2bae7d95 Mon Sep 17 00:00:00 2001 From: Jon Van Oast Date: Tue, 16 Jan 2024 15:55:40 -0700 Subject: [PATCH 08/18] zounds, so much munging --- .../resources/test_collaboration_patch.py | 13 +- .../modules/collaborations/resources/utils.py | 121 ++++++++++++++++-- 2 files changed, 121 insertions(+), 13 deletions(-) diff --git a/tests/modules/collaborations/resources/test_collaboration_patch.py b/tests/modules/collaborations/resources/test_collaboration_patch.py index 497d92833..ec617e9d8 100644 --- a/tests/modules/collaborations/resources/test_collaboration_patch.py +++ b/tests/modules/collaborations/resources/test_collaboration_patch.py @@ -35,10 +35,21 @@ def test_patch_collaboration(flask_app_client, researcher_1, researcher_2, reque ) # Researcher 1 requests that this is escalated to an edit collaboration - collab_utils.request_edit_simple_collaboration( + # now (via #950) this should *not* work as we need export first + resp_msg = 'Unable to start edit on unapproved collaboration' + collab_utils.request_edit(flask_app_client, collab_guid, researcher_1, 400, resp_msg) + + collab_utils.request_export_simple_collaboration( flask_app_client, collab_guid, researcher_1, researcher_2 ) + collab_utils.approve_export_on_collaboration( + flask_app_client, collab_guid, researcher_2, researcher_1 + ) + # now an edit should be possible (as export is approved) + collab_utils.request_edit_simple_collaboration( + flask_app_client, collab_guid, researcher_1, researcher_2 + ) # which is approved collab_utils.approve_edit_on_collaboration( flask_app_client, collab_guid, researcher_2, researcher_1 diff --git a/tests/modules/collaborations/resources/utils.py b/tests/modules/collaborations/resources/utils.py index 201e0103e..b30db3701 100644 --- a/tests/modules/collaborations/resources/utils.py +++ b/tests/modules/collaborations/resources/utils.py @@ -100,6 +100,25 @@ def read_all_collaborations( ) +def request_export( + flask_app_client, + collaboration_guid, + user, + expected_status_code=200, + expected_error=None, +): + return test_utils.post_via_flask( + flask_app_client, + user, + scopes='collaborations:write', + path=f'{PATH}export_request/{collaboration_guid}', + data={}, + expected_status_code=expected_status_code, + response_200={'guid'}, + expected_error=expected_error, + ) + + def request_edit( flask_app_client, collaboration_guid, @@ -136,29 +155,40 @@ def validate_user_access(requesting_user, collab_guid, user_access): collab = get_collab_object_for_user(requesting_user, collab_guid) for user_guid in user_access: assert collab.user_has_read_access(user_guid) == user_access[user_guid]['view'] + assert ( + collab.user_has_export_access(user_guid) == user_access[user_guid]['export'] + ) assert collab.user_has_edit_access(user_guid) == user_access[user_guid]['edit'] def validate_no_access(collab_guid, user_1, user_2): expected_access = { - user_1.guid: {'view': False, 'edit': False}, - user_2.guid: {'view': False, 'edit': False}, + user_1.guid: {'view': False, 'export': False, 'edit': False}, + user_2.guid: {'view': False, 'export': False, 'edit': False}, } validate_user_access(user_1, collab_guid, expected_access) def validate_read_only(collab_guid, user_1, user_2): expected_access = { - user_1.guid: {'view': True, 'edit': False}, - user_2.guid: {'view': True, 'edit': False}, + user_1.guid: {'view': True, 'export': False, 'edit': False}, + user_2.guid: {'view': True, 'export': False, 'edit': False}, + } + validate_user_access(user_1, collab_guid, expected_access) + + +def validate_export_access(collab_guid, user_1, user_2): + expected_access = { + user_1.guid: {'view': True, 'export': True, 'edit': False}, + user_2.guid: {'view': True, 'export': True, 'edit': False}, } validate_user_access(user_1, collab_guid, expected_access) def validate_full_access(collab_guid, user_1, user_2): expected_access = { - user_1.guid: {'view': True, 'edit': True}, - user_2.guid: {'view': True, 'edit': True}, + user_1.guid: {'view': True, 'export': True, 'edit': True}, + user_2.guid: {'view': True, 'export': True, 'edit': True}, } validate_user_access(user_1, collab_guid, expected_access) @@ -229,16 +259,75 @@ def deny_view_on_collaboration(flask_app_client, collab_guid, approving_user, ot return patch_resp +def request_export_simple_collaboration( + flask_app_client, collab_guid, requesting_user, other_user +): + export_resp = request_export(flask_app_client, collab_guid, requesting_user) + expected_states = { + requesting_user.guid: { + 'viewState': 'approved', + 'exportState': 'approved', + 'editState': 'not_initiated', + }, + other_user.guid: { + 'viewState': 'approved', + 'exportState': 'pending', + 'editState': 'not_initiated', + }, + } + validate_expected_states(export_resp.json, expected_states) + validate_read_only(collab_guid, requesting_user, other_user) + + return export_resp + + +def approve_export_on_collaboration( + flask_app_client, collab_guid, approving_user, other_user +): + patch_data = [test_utils.patch_replace_op('export_permission', 'approved')] + + patch_resp = patch_collaboration( + flask_app_client, + collab_guid, + approving_user, + patch_data, + ) + expected_states = { + approving_user.guid: { + 'viewState': 'approved', + 'exportState': 'approved', + 'editState': 'not_initiated', + }, + other_user.guid: { + 'viewState': 'approved', + 'exportState': 'approved', + 'editState': 'not_initiated', + }, + } + validate_expected_states(patch_resp.json, expected_states) + validate_export_access(collab_guid, approving_user, other_user) + + return patch_resp + + def request_edit_simple_collaboration( flask_app_client, collab_guid, requesting_user, other_user ): edit_resp = request_edit(flask_app_client, collab_guid, requesting_user) expected_states = { - requesting_user.guid: {'viewState': 'approved', 'editState': 'approved'}, - other_user.guid: {'viewState': 'approved', 'editState': 'pending'}, + requesting_user.guid: { + 'viewState': 'approved', + 'exportState': 'approved', + 'editState': 'approved', + }, + other_user.guid: { + 'viewState': 'approved', + 'exportState': 'approved', + 'editState': 'pending', + }, } validate_expected_states(edit_resp.json, expected_states) - validate_read_only(collab_guid, requesting_user, other_user) + validate_export_access(collab_guid, requesting_user, other_user) return edit_resp @@ -255,8 +344,16 @@ def approve_edit_on_collaboration( patch_data, ) expected_states = { - approving_user.guid: {'viewState': 'approved', 'editState': 'approved'}, - other_user.guid: {'viewState': 'approved', 'editState': 'approved'}, + approving_user.guid: { + 'viewState': 'approved', + 'exportState': 'approved', + 'editState': 'approved', + }, + other_user.guid: { + 'viewState': 'approved', + 'exportState': 'approved', + 'editState': 'approved', + }, } validate_expected_states(patch_resp.json, expected_states) validate_full_access(collab_guid, approving_user, other_user) @@ -280,7 +377,7 @@ def revoke_edit_on_collaboration( other_user.guid: {'viewState': 'approved', 'editState': 'approved'}, } validate_expected_states(patch_resp.json, expected_states) - validate_read_only(collab_guid, revoking_user, other_user) + validate_export_access(collab_guid, revoking_user, other_user) return patch_resp From 549d61081606666435c675866356adeb7c08fe25 Mon Sep 17 00:00:00 2001 From: Jon Van Oast Date: Tue, 16 Jan 2024 15:57:51 -0700 Subject: [PATCH 09/18] fix --- .../resources/test_collaboration_patch.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/modules/collaborations/resources/test_collaboration_patch.py b/tests/modules/collaborations/resources/test_collaboration_patch.py index ec617e9d8..b0b4de434 100644 --- a/tests/modules/collaborations/resources/test_collaboration_patch.py +++ b/tests/modules/collaborations/resources/test_collaboration_patch.py @@ -112,11 +112,19 @@ def test_view_revoke(flask_app_client, researcher_1, researcher_2, request): flask_app_client, collab_guid, researcher_2, researcher_1 ) + # Researcher 1 requests that this is escalated to an export collaboration + collab_utils.request_export_simple_collaboration( + flask_app_client, collab_guid, researcher_1, researcher_2 + ) + # which is approved + collab_utils.approve_export_on_collaboration( + flask_app_client, collab_guid, researcher_2, researcher_1 + ) + # Researcher 1 requests that this is escalated to an edit collaboration collab_utils.request_edit_simple_collaboration( flask_app_client, collab_guid, researcher_1, researcher_2 ) - # which is approved collab_utils.approve_edit_on_collaboration( flask_app_client, collab_guid, researcher_2, researcher_1 From b751352a72dd575ca340aeeefbe73779d25478e8 Mon Sep 17 00:00:00 2001 From: Jon Van Oast Date: Tue, 16 Jan 2024 17:12:06 -0700 Subject: [PATCH 10/18] whoa a bunch a export/rules/permission tweaking --- app/extensions/__init__.py | 14 ++++++++++++++ app/modules/users/permissions/rules.py | 5 +++++ app/modules/users/permissions/types.py | 1 + 3 files changed, 20 insertions(+) diff --git a/app/extensions/__init__.py b/app/extensions/__init__.py index 6931268aa..4222a228b 100644 --- a/app/extensions/__init__.py +++ b/app/extensions/__init__.py @@ -494,6 +494,13 @@ def current_user_has_view_permission(self): rule = ObjectActionRule(obj=self, action=AccessOperation.READ) return rule.check() + def current_user_has_export_permission(self): + from app.modules.users.permissions.rules import ObjectActionRule + from app.modules.users.permissions.types import AccessOperation + + rule = ObjectActionRule(obj=self, action=AccessOperation.EXPORT) + return rule.check() + def current_user_has_edit_permission(self): from app.modules.users.permissions.rules import ObjectActionRule from app.modules.users.permissions.types import AccessOperation @@ -501,6 +508,13 @@ def current_user_has_edit_permission(self): rule = ObjectActionRule(obj=self, action=AccessOperation.WRITE) return rule.check() + def user_has_export_permission(self, user): + from app.modules.users.permissions.rules import ObjectActionRule + from app.modules.users.permissions.types import AccessOperation + + rule = ObjectActionRule(obj=self, action=AccessOperation.EXPORT, user=user) + return rule.check() + def user_has_view_permission(self, user): from app.modules.users.permissions.rules import ObjectActionRule from app.modules.users.permissions.types import AccessOperation diff --git a/app/modules/users/permissions/rules.py b/app/modules/users/permissions/rules.py index e999f3d72..346ec6af6 100644 --- a/app/modules/users/permissions/rules.py +++ b/app/modules/users/permissions/rules.py @@ -120,11 +120,13 @@ 'is_researcher', ], ('Encounter', AccessOperation.READ): ['is_researcher'], + ('Encounter', AccessOperation.EXPORT): ['is_researcher'], ('Encounter', AccessOperation.WRITE): ['is_active'], # TODO is this still correct ('Sighting', AccessOperation.READ): ['is_researcher'], ('Sighting', AccessOperation.WRITE): ['is_active'], ('Sighting', AccessOperation.DELETE): ['is_admin'], ('Individual', AccessOperation.READ): ['is_researcher'], + ('Individual', AccessOperation.EXPORT): ['is_researcher'], ('Individual', AccessOperation.WRITE): ['is_researcher'], ('Individual', AccessOperation.DELETE): ['is_admin'], ('Annotation', AccessOperation.READ): ['is_researcher'], @@ -197,6 +199,7 @@ # These permissions also granted by collaboration/project/etc so must not be privileged/admin OBJECT_USER_METHOD_MAP = { ('Sighting', AccessOperation.READ): ['user_is_owner'], + ('Sighting', AccessOperation.EXPORT): ['user_is_owner'], ('Sighting', AccessOperation.WRITE): ['user_owns_all_encounters'], ('Sighting', AccessOperation.DELETE): ['user_can_edit_all_encounters'], ('Asset', AccessOperation.READ): ['user_can_access'], @@ -591,6 +594,8 @@ def _permitted_via_collaboration(self, action): if action == AccessOperation.READ: collab_users = Collaboration.get_users_for_read(self._user) + elif action == AccessOperation.EXPORT: + collab_users = Collaboration.get_users_for_export(self._user) elif action == AccessOperation.WRITE: collab_users = Collaboration.get_users_for_write(self._user) else: diff --git a/app/modules/users/permissions/types.py b/app/modules/users/permissions/types.py index d34036604..9d4c2e982 100644 --- a/app/modules/users/permissions/types.py +++ b/app/modules/users/permissions/types.py @@ -23,3 +23,4 @@ class AccessOperation(enum.Enum): # internal operations only (only Sage, no wetware users) WRITE_INTERNAL = 7 DELETE = 8 + EXPORT = 9 From 861245b44aa7e0288a4693d7782534bbf9e27bdf Mon Sep 17 00:00:00 2001 From: Jon Van Oast Date: Tue, 16 Jan 2024 17:15:08 -0700 Subject: [PATCH 11/18] better testing that collabs actually let us do what we think they should do --- .../resources/test_collaboration_usage.py | 64 +++++++++++++++---- 1 file changed, 52 insertions(+), 12 deletions(-) diff --git a/tests/modules/collaborations/resources/test_collaboration_usage.py b/tests/modules/collaborations/resources/test_collaboration_usage.py index 4827aa721..afd490f0c 100644 --- a/tests/modules/collaborations/resources/test_collaboration_usage.py +++ b/tests/modules/collaborations/resources/test_collaboration_usage.py @@ -18,7 +18,7 @@ def test_use_collaboration( flask_app_client, researcher_1, researcher_2, admin_user, test_root, db, request ): - from app.modules.collaborations.models import Collaboration + from app.modules.sightings.models import Sighting uuids = sighting_utils.create_sighting( flask_app_client, researcher_1, request, test_root @@ -27,9 +27,12 @@ def test_use_collaboration( sighting_guid = uuids['sighting'] asset_guid = uuids['assets'][0] enc_guid = uuids['encounters'][0] - data = { - 'user_guid': str(researcher_1.guid), - } + sighting = Sighting.query.get(sighting_guid) + + assert sighting.user_has_view_permission(researcher_1) + assert sighting.user_has_export_permission(researcher_1) + assert not sighting.user_has_view_permission(researcher_2) + assert not sighting.user_has_export_permission(researcher_2) # should not work and should give informative error ags_resp = asset_group_utils.read_asset_group_sighting( @@ -41,15 +44,21 @@ def test_use_collaboration( ) assert ags_resp['message'] == access_error - collab_utils.create_collaboration(flask_app_client, researcher_2, data) - collabs = Collaboration.query.all() - collab = collabs[0] - request.addfinalizer(lambda: collab.delete()) - - asset_group_utils.read_asset_group( - flask_app_client, researcher_2, asset_group_uuid, 403 + # create a (view) collab and approve + create_resp = collab_utils.create_simple_collaboration( + flask_app_client, researcher_1, researcher_2 + ) + collab_guid = create_resp.json['guid'] + collab = collab_utils.get_collab_object_for_user(researcher_1, collab_guid) + request.addfinalizer(collab.delete) + collab_utils.approve_view_on_collaboration( + flask_app_client, collab_guid, researcher_2, researcher_1 ) - collab.set_approval_state_for_user(researcher_1.guid, 'approved') + + assert sighting.user_has_view_permission(researcher_1) + assert sighting.user_has_export_permission(researcher_1) + assert sighting.user_has_view_permission(researcher_2) + assert not sighting.user_has_export_permission(researcher_2) # Researcher 2 should be able to view all the data but edit none of it asset_group_utils.read_asset_group(flask_app_client, researcher_2, asset_group_uuid) @@ -110,3 +119,34 @@ def test_use_collaboration( f'To do this, you need an edit collaboration with {researcher_1.full_name}' ) assert sighting_patch_resp.json['message'] == expected_err + + # Researcher 1 requests that this is escalated to an export collaboration + collab_utils.request_export_simple_collaboration( + flask_app_client, collab_guid, researcher_1, researcher_2 + ) + # which is approved + collab_utils.approve_export_on_collaboration( + flask_app_client, collab_guid, researcher_2, researcher_1 + ) + + assert sighting.user_has_view_permission(researcher_1) + assert sighting.user_has_export_permission(researcher_1) + assert sighting.user_has_view_permission(researcher_2) + assert sighting.user_has_export_permission(researcher_2) + + # Researcher 1 requests that this is escalated to an edit collaboration + collab_utils.request_edit_simple_collaboration( + flask_app_client, collab_guid, researcher_1, researcher_2 + ) + # which is approved + collab_utils.approve_edit_on_collaboration( + flask_app_client, collab_guid, researcher_2, researcher_1 + ) + + # now this should work due to edit collab + sighting_patch_resp = sighting_utils.patch_sighting( + flask_app_client, + researcher_2, + sighting_guid, + sighting_patch, + ) From f744c2bc8e683ed5cdef8f0a296cf4257def4f76 Mon Sep 17 00:00:00 2001 From: Jon Van Oast Date: Tue, 16 Jan 2024 17:24:44 -0700 Subject: [PATCH 12/18] allow edit by doing export first --- .../resources/test_collaboration_notification.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/modules/collaborations/resources/test_collaboration_notification.py b/tests/modules/collaborations/resources/test_collaboration_notification.py index b91f94016..a8ad061a0 100644 --- a/tests/modules/collaborations/resources/test_collaboration_notification.py +++ b/tests/modules/collaborations/resources/test_collaboration_notification.py @@ -46,6 +46,14 @@ def test_edit_collaboration(flask_app_client, researcher_1, researcher_2, db, re flask_app_client, collab_guid, researcher_1, researcher_2 ) + # Researcher 1 requests/approves that this is escalated to an export collaboration (middle step) + collab_utils.request_export_simple_collaboration( + flask_app_client, collab_guid, researcher_1, researcher_2 + ) + collab_utils.approve_export_on_collaboration( + flask_app_client, collab_guid, researcher_2, researcher_1 + ) + # Researcher 2 should now receive a notification researcher_2_notifs = notif_utils.read_all_unread_notifications( flask_app_client, researcher_2 From 03b7b2dd20b699558ddaf9b37df3d637c2864108 Mon Sep 17 00:00:00 2001 From: Jon Van Oast Date: Tue, 16 Jan 2024 17:31:18 -0700 Subject: [PATCH 13/18] insert export into the flow --- tests/modules/collaborations/test_models.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/modules/collaborations/test_models.py b/tests/modules/collaborations/test_models.py index f25fff8c9..13babbaaa 100644 --- a/tests/modules/collaborations/test_models.py +++ b/tests/modules/collaborations/test_models.py @@ -119,8 +119,26 @@ def test_collaboration_edit_state_changes(db, collab_user_a, collab_user_b, requ ) for association in collab.collaboration_user_associations: assert association.read_approval_state == CollaborationUserState.APPROVED + assert association.export_approval_state == CollaborationUserState.NOT_INITIATED assert association.edit_approval_state == CollaborationUserState.NOT_INITIATED + # now export + with mock.patch('app.modules.collaborations.models.current_user', new=collab_user_a): + collab.initiate_export_with_other_user() + + for association in collab.collaboration_user_associations: + if association.user_guid == collab_user_a.guid: + assert association.export_approval_state == CollaborationUserState.APPROVED + if association.user_guid == collab_user_b.guid: + assert association.export_approval_state == CollaborationUserState.PENDING + collab.set_approval_state_for_user( + collab_user_b.guid, CollaborationUserState.APPROVED, level='export' + ) + + for association in collab.collaboration_user_associations: + assert association.export_approval_state == CollaborationUserState.APPROVED + + # now edit with mock.patch('app.modules.collaborations.models.current_user', new=collab_user_a): collab.initiate_edit_with_other_user() From 430e126275a7ee8490b2157146e4c2684f902dce Mon Sep 17 00:00:00 2001 From: Jon Van Oast Date: Wed, 17 Jan 2024 11:54:55 -0700 Subject: [PATCH 14/18] exporter_guids() parallel to viewer_guids() --- app/extensions/__init__.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/app/extensions/__init__.py b/app/extensions/__init__.py index 4222a228b..57b321a2f 100644 --- a/app/extensions/__init__.py +++ b/app/extensions/__init__.py @@ -536,6 +536,20 @@ def viewer_guids(self): vguids.append(str(user.guid)) return vguids + def exporter_guids(self): + from app.modules.users.models import User + + users = User.query.all() + vguids = [] + if self.is_public(): + for user in users: + vguids.append(str(user.guid)) + else: + for user in users: + if user.is_admin or self.user_has_export_permission(user): + vguids.append(str(user.guid)) + return vguids + def get_all_owners(self): if hasattr(self, 'owner'): return [getattr(self, 'owner')] From 18b76e8ea23de4d7e47f96657bbb1d91a11da945 Mon Sep 17 00:00:00 2001 From: Jon Van Oast Date: Wed, 17 Jan 2024 12:23:53 -0700 Subject: [PATCH 15/18] skip internal users --- app/extensions/__init__.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/extensions/__init__.py b/app/extensions/__init__.py index 57b321a2f..4b5d91c0e 100644 --- a/app/extensions/__init__.py +++ b/app/extensions/__init__.py @@ -532,7 +532,9 @@ def viewer_guids(self): vguids.append(str(user.guid)) else: for user in users: - if user.is_admin or self.user_has_view_permission(user): + if not user.is_internal and ( + user.is_admin or self.user_has_view_permission(user) + ): vguids.append(str(user.guid)) return vguids @@ -546,7 +548,9 @@ def exporter_guids(self): vguids.append(str(user.guid)) else: for user in users: - if user.is_admin or self.user_has_export_permission(user): + if not user.is_internal and ( + user.is_admin or self.user_has_export_permission(user) + ): vguids.append(str(user.guid)) return vguids From deb4d53cb31f6be6e26a6af8aceaf5d585ce10b4 Mon Sep 17 00:00:00 2001 From: Jon Van Oast Date: Wed, 17 Jan 2024 16:32:05 -0700 Subject: [PATCH 16/18] move search header from viewers to exporters --- app/extensions/api/namespace.py | 11 ++-- app/modules/encounters/resources.py | 24 +++---- app/modules/encounters/schemas.py | 4 +- app/modules/individuals/resources.py | 20 +++--- app/modules/individuals/schemas.py | 4 +- app/modules/sightings/resources.py | 20 +++--- app/modules/sightings/schemas.py | 4 +- .../resources/test_sighting_elasticsearch.py | 65 +++++++++++++++++-- 8 files changed, 109 insertions(+), 43 deletions(-) diff --git a/app/extensions/api/namespace.py b/app/extensions/api/namespace.py index c3607aedb..30adee1a6 100644 --- a/app/extensions/api/namespace.py +++ b/app/extensions/api/namespace.py @@ -135,7 +135,7 @@ def wrapper(self_, parameters_args, *args, **kwargs): query = func(self_, parameters_args, *args, **kwargs) - viewable_count = -1 + exportable_count = -1 if not isinstance(query, flask_sqlalchemy.BaseQuery): if query is None or len(query) == 0: total_count, response = 0, [] @@ -143,9 +143,9 @@ def wrapper(self_, parameters_args, *args, **kwargs): total_count, response = query assert isinstance(total_count, int) elif len(query) == 3: - total_count, response, viewable_count = query + total_count, response, exportable_count = query assert isinstance(total_count, int) - assert isinstance(viewable_count, int) + assert isinstance(exportable_count, int) else: raise ValueError( 'This may happen when @api.paginate is above @api.response' @@ -239,7 +239,10 @@ def wrapper(self_, parameters_args, *args, **kwargs): return ( response, HTTPStatus.OK, - {'X-Total-Count': total_count, 'X-Viewable-Count': viewable_count}, + { + 'X-Total-Count': total_count, + 'X-Exportable-Count': exportable_count, + }, ) return self.parameters(parameters, locations)(wrapper) diff --git a/app/modules/encounters/resources.py b/app/modules/encounters/resources.py index 4b82b6585..a88209bc4 100644 --- a/app/modules/encounters/resources.py +++ b/app/modules/encounters/resources.py @@ -80,10 +80,10 @@ def post(self, args): search = request.get_json() args['total'] = True - # hacky way to skip when already querying on viewers or query is "unusual"(?) + # hacky way to skip when already querying on exporters or query is "unusual"(?) if ( not current_user - or '"viewers"' in str(search) + or '"exporters"' in str(search) or 'bool' not in search or 'filter' not in search['bool'] or not isinstance(search['bool']['filter'], list) @@ -91,13 +91,15 @@ def post(self, args): return Encounter.elasticsearch(search, **args) from copy import deepcopy - view_search = deepcopy(search) - view_search['bool']['filter'].append( - {'match': {'viewers': str(current_user.guid)}} + export_search = deepcopy(search) + export_search['bool']['filter'].append( + {'match': {'exporters': str(current_user.guid)}} ) - log.debug(f'doing viewer search using {view_search}') - view_count, view_res = Encounter.elasticsearch(view_search, load=False, **args) - return Encounter.elasticsearch(search, **args) + (view_count,) + log.debug(f'doing exporters search using {export_search}') + export_count, export_res = Encounter.elasticsearch( + export_search, load=False, **args + ) + return Encounter.elasticsearch(search, **args) + (export_count,) @api.route('/') @@ -258,20 +260,20 @@ def post(self): added = set() export = Export() for enc in encs: - if not enc.current_user_has_view_permission(): + if not enc.current_user_has_export_permission(): continue export.add(enc) ct += 1 if ( enc.sighting_guid not in added - and enc.sighting.current_user_has_view_permission() + and enc.sighting.current_user_has_export_permission() ): added.add(enc.sighting_guid) export.add(enc.sighting) if ( enc.individual_guid and enc.individual_guid not in added - and enc.individual.current_user_has_view_permission() + and enc.individual.current_user_has_export_permission() ): added.add(enc.individual_guid) export.add(enc.individual) diff --git a/app/modules/encounters/schemas.py b/app/modules/encounters/schemas.py index e28197a52..1c496137c 100644 --- a/app/modules/encounters/schemas.py +++ b/app/modules/encounters/schemas.py @@ -62,7 +62,7 @@ class ElasticsearchEncounterSchema(ModelSchema): taxonomy_guid = base_fields.Function( lambda enc: enc.get_taxonomy_guid_no_fallback_str() ) - viewers = base_fields.Function(lambda enc: enc.viewer_guids()) + exporters = base_fields.Function(lambda enc: enc.exporter_guids()) class Meta: # pylint: disable=missing-docstring @@ -90,7 +90,7 @@ class Meta: 'location_geo_point', 'customFields', 'hasView', - 'viewers', + 'exporters', ) dump_only = (Encounter.guid.key,) diff --git a/app/modules/individuals/resources.py b/app/modules/individuals/resources.py index 8608e2a60..f0fad8e2c 100644 --- a/app/modules/individuals/resources.py +++ b/app/modules/individuals/resources.py @@ -238,10 +238,10 @@ def post(self, args): search = request.get_json() args['total'] = True - # hacky way to skip when already querying on viewers or query is "unusual"(?) + # hacky way to skip when already querying on exporters or query is "unusual"(?) if ( not current_user - or '"viewers"' in str(search) + or '"exporters"' in str(search) or 'bool' not in search or 'filter' not in search['bool'] or not isinstance(search['bool']['filter'], list) @@ -249,13 +249,15 @@ def post(self, args): return Individual.elasticsearch(search, **args) from copy import deepcopy - view_search = deepcopy(search) - view_search['bool']['filter'].append( - {'match': {'viewers': str(current_user.guid)}} + export_search = deepcopy(search) + export_search['bool']['filter'].append( + {'match': {'exporters': str(current_user.guid)}} ) - log.debug(f'doing viewer search using {view_search}') - view_count, view_res = Individual.elasticsearch(view_search, load=False, **args) - return Individual.elasticsearch(search, **args) + (view_count,) + log.debug(f'doing exporters search using {export_search}') + export_count, export_res = Individual.elasticsearch( + export_search, load=False, **args + ) + return Individual.elasticsearch(search, **args) + (export_count,) @api.route('/export') @@ -280,7 +282,7 @@ def post(self): ct = 0 export = Export() for indiv in indivs: - if not indiv.current_user_has_view_permission(): + if not indiv.current_user_has_export_permission(): continue export.add(indiv) ct += 1 diff --git a/app/modules/individuals/schemas.py b/app/modules/individuals/schemas.py index 1586f9a67..3bf5188a1 100644 --- a/app/modules/individuals/schemas.py +++ b/app/modules/individuals/schemas.py @@ -201,7 +201,7 @@ class ElasticsearchIndividualSchema(ModelSchema): lambda ind: ind.get_taxonomy_names(inherit_encounters=False) ) numberSightings = base_fields.Function(lambda ind: ind.get_number_sightings()) - viewers = base_fields.Function(lambda ind: ind.viewer_guids()) + exporters = base_fields.Function(lambda ind: ind.exporter_guids()) class Meta: # pylint: disable=missing-docstring @@ -233,7 +233,7 @@ class Meta: 'last_seen_specificity', 'taxonomy_names', 'numberSightings', - 'viewers', + 'exporters', ) dump_only = ( Individual.guid.key, diff --git a/app/modules/sightings/resources.py b/app/modules/sightings/resources.py index 6135da5be..b22aca7b9 100644 --- a/app/modules/sightings/resources.py +++ b/app/modules/sightings/resources.py @@ -115,10 +115,10 @@ def get(self, args): def post(self, args): search = request.get_json() args['total'] = True - # hacky way to skip when already querying on viewers or query is "unusual"(?) + # hacky way to skip when already querying on exporters or query is "unusual"(?) if ( not current_user - or '"viewers"' in str(search) + or '"exporters"' in str(search) or 'bool' not in search or 'filter' not in search['bool'] or not isinstance(search['bool']['filter'], list) @@ -126,13 +126,15 @@ def post(self, args): return Sighting.elasticsearch(search, **args) from copy import deepcopy - view_search = deepcopy(search) - view_search['bool']['filter'].append( - {'match': {'viewers': str(current_user.guid)}} + export_search = deepcopy(search) + export_search['bool']['filter'].append( + {'match': {'exporters': str(current_user.guid)}} ) - log.debug(f'doing viewer search using {view_search}') - view_count, view_res = Sighting.elasticsearch(view_search, load=False, **args) - return Sighting.elasticsearch(search, **args) + (view_count,) + log.debug(f'doing exporters search using {export_search}') + export_count, export_res = Sighting.elasticsearch( + export_search, load=False, **args + ) + return Sighting.elasticsearch(search, **args) + (export_count,) @api.route('/export') @@ -157,7 +159,7 @@ def post(self): export = Export() ct = 0 for sight in sights: - if not sight.current_user_has_view_permission(): + if not sight.current_user_has_export_permission(): continue ct += 1 export.add(sight) diff --git a/app/modules/sightings/schemas.py b/app/modules/sightings/schemas.py index d1f36a014..4498572b4 100644 --- a/app/modules/sightings/schemas.py +++ b/app/modules/sightings/schemas.py @@ -131,10 +131,10 @@ class Meta: class ElasticsearchSightingSchema(ElasticsearchSightingReturnSchema): - viewers = base_fields.Function(lambda s: s.viewer_guids()) + exporters = base_fields.Function(lambda s: s.exporter_guids()) class Meta(ElasticsearchSightingReturnSchema.Meta): - fields = ElasticsearchSightingReturnSchema.Meta.fields + ('viewers',) + fields = ElasticsearchSightingReturnSchema.Meta.fields + ('exporters',) class TimedSightingSchema(CreateSightingSchema): diff --git a/tests/modules/sightings/resources/test_sighting_elasticsearch.py b/tests/modules/sightings/resources/test_sighting_elasticsearch.py index 2c2e46428..b1dde4942 100644 --- a/tests/modules/sightings/resources/test_sighting_elasticsearch.py +++ b/tests/modules/sightings/resources/test_sighting_elasticsearch.py @@ -45,7 +45,8 @@ def no_test_sighting_elasticsearch_mappings(flask_app_client, researcher_1): @pytest.mark.skipif( test_utils.module_unavailable('sightings'), reason='Sightings module disabled' ) -def test_search(flask_app_client, researcher_1, request, test_root): +def test_search(flask_app_client, researcher_1, researcher_2, request, test_root): + import tests.modules.collaborations.resources.utils as collab_utils from app.modules.sightings.models import Sighting sighting_guid = sighting_utils.create_sighting( @@ -55,9 +56,21 @@ def test_search(flask_app_client, researcher_1, request, test_root): test_root, )['sighting'] # Force created sighting to be indexed in elasticsearch - Sighting.query.get(sighting_guid).index() + sighting = Sighting.query.get(sighting_guid) + sighting.index() wait_for_elasticsearch_status(flask_app_client, researcher_1) + # create a view-only collab between these two + create_resp = collab_utils.create_simple_collaboration( + flask_app_client, researcher_1, researcher_2 + ) + collab_guid = create_resp.json['guid'] + collab = collab_utils.get_collab_object_for_user(researcher_1, collab_guid) + request.addfinalizer(collab.delete) + collab_utils.approve_view_on_collaboration( + flask_app_client, collab_guid, researcher_2, researcher_1 + ) + resp = test_utils.get_list_via_flask( flask_app_client, researcher_1, @@ -70,7 +83,7 @@ def test_search(flask_app_client, researcher_1, request, test_root): assert resp.json[0]['guid'] == str(sighting_guid) assert resp.headers['X-Total-Count'] == '1' # is -1 cuz query above was "atypical" .... meh - assert resp.headers['X-Viewable-Count'] == '-1' + assert resp.headers['X-Exportable-Count'] == '-1' resp = test_utils.post_via_flask( flask_app_client, @@ -85,7 +98,51 @@ def test_search(flask_app_client, researcher_1, request, test_root): assert len(resp.json) == 1 assert resp.json[0]['guid'] == str(sighting_guid) assert resp.headers['X-Total-Count'] == '1' - assert resp.headers['X-Viewable-Count'] == '1' + assert resp.headers['X-Exportable-Count'] == '1' + + # researcher_2 does NOT have export collab, so exportable-count should be zero + resp = test_utils.post_via_flask( + flask_app_client, + researcher_2, + scopes='sightings:read', + path='/api/v1/sightings/search', + data={'bool': {'filter': [], 'must_not': []}}, + expected_status_code=200, + response_200=EXPECTED_KEYS, + returns_list=True, + ) + assert len(resp.json) == 1 + assert resp.json[0]['guid'] == str(sighting_guid) + assert resp.headers['X-Total-Count'] == '1' + assert resp.headers['X-Exportable-Count'] == '0' + + # set up export collab between them now + collab_utils.request_export_simple_collaboration( + flask_app_client, collab_guid, researcher_1, researcher_2 + ) + collab_utils.approve_export_on_collaboration( + flask_app_client, collab_guid, researcher_2, researcher_1 + ) + + # FIXME this should be done automagically (in the future) when collab is set up :( + sighting.index() + wait_for_elasticsearch_status(flask_app_client, researcher_1) + + # now researcher_2 should be able to see sighting as exportable + resp = test_utils.post_via_flask( + flask_app_client, + researcher_2, + scopes='sightings:read', + path='/api/v1/sightings/search', + data={'bool': {'filter': [], 'must_not': []}}, + expected_status_code=200, + response_200=EXPECTED_KEYS, + returns_list=True, + ) + assert len(resp.json) == 1 + assert resp.json[0]['guid'] == str(sighting_guid) + assert resp.headers['X-Total-Count'] == '1' + assert resp.headers['X-Exportable-Count'] == '1' @pytest.mark.skipif(module_unavailable('sightings'), reason='Sightings module disabled') From c09ca0fd618cfbee593d60321843bed75738086e Mon Sep 17 00:00:00 2001 From: Jon Van Oast Date: Thu, 18 Jan 2024 11:15:08 -0700 Subject: [PATCH 17/18] handle quasi-migration of collab states for existing collabs --- migrations/versions/effd65fb089e_.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/migrations/versions/effd65fb089e_.py b/migrations/versions/effd65fb089e_.py index 53afff97f..96c3c8c71 100644 --- a/migrations/versions/effd65fb089e_.py +++ b/migrations/versions/effd65fb089e_.py @@ -50,9 +50,31 @@ def upgrade(): ['guid'], ) + # we set with nullable=True since we likely have rows in this already, then toggle after with op.batch_alter_table('collaboration_user_associations', schema=None) as batch_op: batch_op.add_column( - sa.Column('export_approval_state', sa.String(length=32), nullable=False) + sa.Column('export_approval_state', sa.String(length=32), nullable=True) + ) + # we set export_approval_state now so we can make it not-null after + op.execute( + "UPDATE collaboration_user_associations SET export_approval_state='not_initiated'" + ) + # if a collab is already in the edit=approved state, we just let them get export as well + op.execute( + "UPDATE collaboration_user_associations SET export_approval_state='approved' WHERE edit_approval_state='approved'" + ) + # any other state for edit just gets pushed down to export, then edit reset to not_initiated + # (note: yes this will move not_initiated too, but thats fine; just makes the sql simpler) + op.execute( + "UPDATE collaboration_user_associations SET export_approval_state=edit_approval_state WHERE edit_approval_state!='approved'" + ) + op.execute( + "UPDATE collaboration_user_associations SET edit_approval_state='not_initiated' WHERE edit_approval_state!='approved'" + ) + # and here we set nullable=False once we have values in + with op.batch_alter_table('collaboration_user_associations', schema=None) as batch_op: + batch_op.alter_column( + 'export_approval_state', existing_type=sa.VARCHAR, nullable=False ) # ### end Alembic commands ### From b25e871adfc5a9b51f7a7cff8d4430e5e176c331 Mon Sep 17 00:00:00 2001 From: Jon Van Oast Date: Mon, 29 Jan 2024 11:17:10 -0700 Subject: [PATCH 18/18] resolve db migration collision --- migrations/versions/effd65fb089e_.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/migrations/versions/effd65fb089e_.py b/migrations/versions/effd65fb089e_.py index 96c3c8c71..8db69aa27 100644 --- a/migrations/versions/effd65fb089e_.py +++ b/migrations/versions/effd65fb089e_.py @@ -2,7 +2,7 @@ """empty message Revision ID: effd65fb089e -Revises: 53b0fec87272 +Revises: dc91b517a7a4 Create Date: 2024-01-05 18:24:21.360901 """ @@ -14,7 +14,7 @@ # revision identifiers, used by Alembic. revision = 'effd65fb089e' -down_revision = '53b0fec87272' +down_revision = 'dc91b517a7a4' def upgrade():