Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Refactor notifications from core to modules #822

Merged
merged 24 commits into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
2635aa5
Move notifications block in config + frontend changes
dlpzx Oct 20, 2023
34a88fa
Backend changes notifications to modules
dlpzx Oct 20, 2023
5006ac3
Adapt and fix tests for notifications
dlpzx Oct 20, 2023
a5d6eaf
Add migration and fix tests
dlpzx Oct 20, 2023
3bafb83
Add migration script and fix references in SES stack
dlpzx Oct 20, 2023
ee2cc20
Fix SES stack identity
dlpzx Oct 23, 2023
7a3e481
Account principal in KMS key for SNS in SES
dlpzx Oct 23, 2023
24861a7
Move checking local into notifications handler
dlpzx Oct 23, 2023
5695fae
Added comments
dlpzx Oct 23, 2023
bab89bd
Clean-up notification class for UI notifications
dlpzx Oct 23, 2023
e878bb0
Modifying checks for ses stack creation
dlpzx Oct 23, 2023
d8fa1a4
Changes in ShareNotification invokations
dlpzx Oct 23, 2023
d138602
Replace email recipient from context to share.owner. Remove email_id …
dlpzx Oct 24, 2023
dc39d89
Fix tests
dlpzx Oct 24, 2023
62468f8
change config.json default, better check conditions in deploy and ser…
dlpzx Oct 25, 2023
55364f8
Rename Notification.username to recipient (groups and usernames), add…
dlpzx Oct 25, 2023
580750e
Comment unused queries
dlpzx Oct 25, 2023
01e2db0
Avoid duplicated notifications if stewards=owners
dlpzx Oct 25, 2023
9d59198
Make config.json more standard
dlpzx Oct 25, 2023
b902f8b
Merge branch 'v2m1m0' into feat/refactor-notifications-to-modules
dlpzx Oct 25, 2023
612ec58
Move email_sender_id inside custom_domain. Modify conditions
dlpzx Oct 25, 2023
ccde999
Merge remote-tracking branch 'origin/feat/refactor-notifications-to-m…
dlpzx Oct 25, 2023
5907c6b
Typo in config.json
dlpzx Oct 25, 2023
4e9a05b
Small typo + local development mocking in SES and Cognito
dlpzx Oct 26, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions backend/api_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ def handler(event, context):

if 'authorizer' in event['requestContext']:
username = event['requestContext']['authorizer']['claims']['email']
email_id = event['requestContext']['authorizer']['claims']['email']
try:
groups = get_groups(event['requestContext']['authorizer']['claims'])
with ENGINE.scoped_session() as session:
Expand All @@ -138,14 +137,13 @@ def handler(event, context):
print(f'Error managing groups due to: {e}')
groups = []

set_context(RequestContext(ENGINE, username, email_id, groups))
set_context(RequestContext(ENGINE, username, groups))

app_context = {
'engine': ENGINE,
'username': username,
'groups': groups,
'schema': SCHEMA,
'emailId': email_id,
'schema': SCHEMA
}

else:
Expand Down
6 changes: 4 additions & 2 deletions backend/dataall/base/aws/cognito.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ def __init__(self):
def get_user_emailids_from_group(self, groupName):
try:
envname = os.getenv('envname', 'local')
if envname == 'local' or envname == 'dkrcompose':
dlpzx marked this conversation as resolved.
Show resolved Hide resolved
raise Exception('Cognito is not supported in local dev environment')
parameter_path = f'/dataall/{envname}/cognito/userpool'
ssm = boto3.client('ssm', region_name=os.getenv('AWS_REGION', 'eu-west-1'))
user_pool_id = ssm.get_parameter(Name=parameter_path)['Parameter']['Value']
Expand All @@ -29,6 +27,10 @@ def get_user_emailids_from_group(self, groupName):
group_email_ids.extend([x['Value'] for x in attributes if x['Name'] == 'email'])

except Exception as e:
envname = os.getenv('envname', 'local')
if envname in ['local', 'dkrcompose']:
log.error('Local development environment does not support Cognito')
return ['[email protected]']
log.error(
f'Failed to get email ids for Cognito group {groupName} due to {e}'
)
Expand Down
4 changes: 4 additions & 0 deletions backend/dataall/base/aws/ses.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,9 @@ def send_email(self, toList, message, subject):
}
)
except Exception as e:
envname = os.getenv('envname', 'local')
if envname in ['local', 'dkrcompose']:
log.error('Local development environment does not support SES notifications')
return True
log.error(f'Error while sending email {e})')
raise e
1 change: 0 additions & 1 deletion backend/dataall/base/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ class RequestContext:
"""Contains API for every graphql request"""
db_engine: Engine
username: str
email_id: str
groups: List[str]


Expand Down
1 change: 0 additions & 1 deletion backend/dataall/core/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
from dataall.core import (
permissions,
stacks,
notifications,
cognito_groups,
environment,
organizations,
Expand Down
1 change: 0 additions & 1 deletion backend/dataall/core/notifications/__init__.py

This file was deleted.

9 changes: 0 additions & 9 deletions backend/dataall/core/notifications/api/__init__.py

This file was deleted.

48 changes: 0 additions & 48 deletions backend/dataall/core/notifications/api/resolvers.py

This file was deleted.

3 changes: 0 additions & 3 deletions backend/dataall/core/notifications/handlers/__init__.py

This file was deleted.

3 changes: 1 addition & 2 deletions backend/dataall/core/permissions/api/types.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
from dataall.base.api import gql
from dataall.core.notifications.db.notification_models import Notification
from dataall.core.permissions.db.permission_models import PermissionType


def resolve_enum(context, source: Notification):
def resolve_enum(context, source: PermissionType):
return source.type.name if source.type else PermissionType.TENANT.name


Expand Down
6 changes: 4 additions & 2 deletions backend/dataall/modules/dataset_sharing/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ def is_supported(modes: Set[ImportMode]) -> bool:

@staticmethod
def depends_on() -> List[Type['ModuleInterface']]:
return [DatasetBaseModuleInterface]
from dataall.modules.notifications import NotificationsModuleInterface
return [DatasetBaseModuleInterface, NotificationsModuleInterface]

def __init__(self):
from dataall.modules.dataset_sharing import api
Expand All @@ -35,7 +36,8 @@ def is_supported(modes: List[ImportMode]):

@staticmethod
def depends_on() -> List[Type['ModuleInterface']]:
return [DatasetBaseModuleInterface]
from dataall.modules.notifications import NotificationsModuleInterface
return [DatasetBaseModuleInterface, NotificationsModuleInterface]

def __init__(self):
import dataall.modules.dataset_sharing.handlers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,11 @@ def revoke_items_share_object(uri, revoked_uris):
resource_uri=dataset.datasetUri,
)

ShareNotificationService.notify_share_object_rejection(session, context.username, context.email_id, dataset, share)
ShareNotificationService(
session=session,
dataset=dataset,
share=share
).notify_share_object_rejection(email_id=context.username)
dlpzx marked this conversation as resolved.
Show resolved Hide resolved

revoke_share_task: Task = Task(
action='ecs.share.revoke',
Expand Down
Loading