-
Notifications
You must be signed in to change notification settings - Fork 21
DEP-265: Remove engagement slug table; merge into engagements #2876
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
Changes from all commits
4674f0a
5274f9c
4a8db7d
3b63c35
adda3e5
f82259a
c4bbaf8
0f94440
5fe5d9e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| """Move slug from separate table to engagement table | ||
|
|
||
| Revision ID: e572f7d709c5 | ||
| Revises: e1f7a3c9b2d5 | ||
| Create Date: 2026-06-15 14:06:46.186253 | ||
|
|
||
| """ | ||
| from alembic import op | ||
| import sqlalchemy as sa | ||
| from sqlalchemy.dialects import postgresql | ||
|
|
||
| # revision identifiers, used by Alembic. | ||
| revision = 'e572f7d709c5' | ||
| down_revision = 'e1f7a3c9b2d5' | ||
| branch_labels = None | ||
| depends_on = None | ||
|
|
||
|
|
||
| def upgrade(): | ||
| op.drop_index('idx_slug', table_name='engagement_slug') | ||
| op.add_column('engagement', sa.Column( | ||
| 'slug', sa.String(length=256), nullable=True)) | ||
| # Migrate data from engagement_slug to engagement | ||
| op.execute(""" | ||
| UPDATE engagement e | ||
| SET slug = es.slug | ||
| FROM engagement_slug es | ||
| WHERE e.id = es.engagement_id | ||
| """) | ||
| # If there are any engagements without a slug, auto-generate from the name | ||
| op.execute(""" | ||
| UPDATE engagement | ||
| SET slug = regexp_replace(lower(name), '\\s+', '-', 'g') | ||
| WHERE slug IS NULL | ||
| """) | ||
| op.alter_column('engagement', 'slug', | ||
| existing_type=sa.String(length=256), | ||
| nullable=False) | ||
| op.create_unique_constraint( | ||
| 'uq_slug_per_tenant', 'engagement', ['slug', 'tenant_id']) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice, so a different tenant can use the same slug |
||
| op.drop_table('engagement_slug') | ||
|
|
||
|
|
||
| def downgrade(): | ||
| op.drop_constraint('uq_slug_per_tenant', 'engagement', type_='unique') | ||
| op.drop_column('engagement', 'slug') | ||
| op.create_table('engagement_slug', | ||
| sa.Column('created_date', postgresql.TIMESTAMP(), | ||
| autoincrement=False, nullable=False), | ||
| sa.Column('updated_date', postgresql.TIMESTAMP(), | ||
| autoincrement=False, nullable=True), | ||
| sa.Column('id', sa.INTEGER(), | ||
| autoincrement=True, nullable=False), | ||
| sa.Column('engagement_id', sa.INTEGER(), | ||
| autoincrement=False, nullable=False), | ||
| sa.Column('slug', sa.VARCHAR(length=200), | ||
| autoincrement=False, nullable=False), | ||
| sa.Column('created_by', sa.VARCHAR(length=50), | ||
| autoincrement=False, nullable=True), | ||
| sa.Column('updated_by', sa.VARCHAR(length=50), | ||
| autoincrement=False, nullable=True), | ||
| sa.ForeignKeyConstraint(['engagement_id'], [ | ||
| 'engagement.id'], name='engagement_slug_engagement_id_fkey', ondelete='CASCADE'), | ||
| sa.PrimaryKeyConstraint('id', name='engagement_slug_pkey'), | ||
| sa.UniqueConstraint( | ||
| 'engagement_id', name='engagement_slug_engagement_id_key'), | ||
| sa.UniqueConstraint( | ||
| 'slug', name='engagement_slug_slug_key') | ||
| ) | ||
| op.create_index('idx_slug', 'engagement_slug', ['slug'], unique=False) | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,7 +46,6 @@ | |
| from .widget_map import API as WIDGET_MAPS_API | ||
| from .shape_file import API as SHAPEFILE_API | ||
| from .tenant import API as TENANT_API | ||
| from .engagement_slug import API as ENGAGEMENT_SLUG_API | ||
| from .report_setting import API as REPORT_SETTING_API | ||
| from .widget_video import API as WIDGET_VIDEO_API | ||
| from .engagement_settings import API as ENGAGEMENT_SETTINGS_API | ||
|
|
@@ -85,35 +84,49 @@ | |
| API.add_namespace(SUBSCRIPTION_API) | ||
| API.add_namespace(COMMENT_API) | ||
| API.add_namespace(EMAIL_VERIFICATION_API) | ||
| API.add_namespace(ENGAGEMENT_DETAILS_TAB_API, path='/engagement/<int:engagement_id>/details') | ||
| API.add_namespace(ENGAGEMENT_DETAILS_TAB_API, | ||
| path='/engagement/<int:engagement_id>/details') | ||
| API.add_namespace(FEEDBACK_API) | ||
| API.add_namespace(WIDGET_API) | ||
| API.add_namespace(CONTACT_API) | ||
| API.add_namespace(VALUE_COMPONENTS_API) | ||
| API.add_namespace(SHAPEFILE_API) | ||
| API.add_namespace(TENANT_API) | ||
| API.add_namespace(METADATA_TAXON_API, path='/engagement_metadata') | ||
| API.add_namespace(ENGAGEMENT_METADATA_API, path='/engagements/<int:engagement_id>/metadata') | ||
| API.add_namespace(ENGAGEMENT_MEMBERS_API, path='/engagements/<string:engagement_id>/members') | ||
| API.add_namespace(WIDGET_DOCUMENTS_API, path='/widgets/<string:widget_id>/documents') | ||
| API.add_namespace(ENGAGEMENT_METADATA_API, | ||
| path='/engagements/<int:engagement_id>/metadata') | ||
| API.add_namespace(ENGAGEMENT_MEMBERS_API, | ||
| path='/engagements/<string:engagement_id>/members') | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just noticing that engagement_id is defined as a string or an integer for different endpoints, I'm guessing that's intentional. |
||
| API.add_namespace(WIDGET_DOCUMENTS_API, | ||
| path='/widgets/<string:widget_id>/documents') | ||
| API.add_namespace(WIDGET_EVENTS_API, path='/widgets/<int:widget_id>/events') | ||
| API.add_namespace(WIDGET_MAPS_API, path='/widgets/<int:widget_id>/maps') | ||
| API.add_namespace(ENGAGEMENT_SLUG_API, path='/slugs') | ||
| API.add_namespace(REPORT_SETTING_API, path='/surveys/<int:survey_id>/reportsettings') | ||
| API.add_namespace(REPORT_SETTING_API, | ||
| path='/surveys/<int:survey_id>/reportsettings') | ||
| API.add_namespace(WIDGET_VIDEO_API, path='/widgets/<int:widget_id>/videos') | ||
| API.add_namespace(WIDGET_LISTENING_API, path='/widgets/<int:widget_id>/listening_widgets') | ||
| API.add_namespace(WIDGET_LISTENING_API, | ||
| path='/widgets/<int:widget_id>/listening_widgets') | ||
| API.add_namespace(ENGAGEMENT_SETTINGS_API) | ||
| API.add_namespace(WIDGET_TIMELINE_API, path='/widgets/<int:widget_id>/timelines') | ||
| API.add_namespace(WIDGET_TIMELINE_API, | ||
| path='/widgets/<int:widget_id>/timelines') | ||
| API.add_namespace(WIDGET_POLL_API, path='/widgets/<int:widget_id>/polls') | ||
| API.add_namespace(WIDGET_IMAGE_API, path='/widgets/<int:widget_id>/images') | ||
| API.add_namespace(LANGUAGE_API, path='/languages') | ||
| API.add_namespace(SURVEY_TRANSLATION_API, path='/surveys/<int:survey_id>/translations') | ||
| API.add_namespace(POLL_ANSWER_TRANSLATION_API, path='/polls/<int:poll_id>/translations') | ||
| API.add_namespace(SURVEY_TRANSLATION_API, | ||
| path='/surveys/<int:survey_id>/translations') | ||
| API.add_namespace(POLL_ANSWER_TRANSLATION_API, | ||
| path='/polls/<int:poll_id>/translations') | ||
| API.add_namespace(CONTACT_TRANSLATION_API, path='/contact_translations') | ||
| API.add_namespace(EVENT_ITEM_TRANSLATION_API, path='/events/<int:event_id>/translations') | ||
| API.add_namespace(TIMELINE_EVENT_TRANSLATION_API, path='/timelines/<int:timeline_id>/translations') | ||
| API.add_namespace(ENGAGEMENT_TRANSLATION_API, path='/engagement/<int:engagement_id>/translations') | ||
| API.add_namespace(ENGAGEMENT_DETAILS_TAB_TRANSLATION_API, path='/engagement/<int:engagement_id>/details/translations') | ||
| API.add_namespace(ENGAGEMENT_CONTENT_TRANSLATION_API, path='/engagement/<int:engagement_id>/content/translations') | ||
| API.add_namespace(EVENT_ITEM_TRANSLATION_API, | ||
| path='/events/<int:event_id>/translations') | ||
| API.add_namespace(TIMELINE_EVENT_TRANSLATION_API, | ||
| path='/timelines/<int:timeline_id>/translations') | ||
| API.add_namespace(ENGAGEMENT_TRANSLATION_API, | ||
| path='/engagement/<int:engagement_id>/translations') | ||
| API.add_namespace(ENGAGEMENT_DETAILS_TAB_TRANSLATION_API, | ||
| path='/engagement/<int:engagement_id>/details/translations') | ||
| API.add_namespace(ENGAGEMENT_CONTENT_TRANSLATION_API, | ||
| path='/engagement/<int:engagement_id>/content/translations') | ||
| API.add_namespace(VERSION_API, path='/version') | ||
| API.add_namespace(widget_image_translation_api, path='/widget-image-translation') | ||
| API.add_namespace(widget_image_translation_api, | ||
| path='/widget-image-translation') | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,29 +13,33 @@ | |
| # limitations under the License. | ||
| """API endpoints for managing an engagement resource.""" | ||
|
|
||
| import json | ||
| from http import HTTPStatus | ||
|
|
||
| import json | ||
| from flask import request | ||
| from flask import current_app, request | ||
| from flask_cors import cross_origin | ||
| from flask_restx import Namespace, Resource | ||
| from marshmallow import ValidationError | ||
|
|
||
| from api.auth import auth | ||
| from api.auth import jwt as _jwt | ||
| from api.models.pagination_options import PaginationOptions | ||
| from api.models.tenant import Tenant as TenantModel | ||
| from api.schemas.engagement import EngagementSchema | ||
| from api.services.engagement_service import EngagementService | ||
| from api.utils.roles import Role | ||
| from api.utils.tenant_validator import require_role | ||
| from api.utils.token_info import TokenInfo | ||
| from api.utils.util import allowedorigins, cors_preflight | ||
|
|
||
|
|
||
| API = Namespace( | ||
| 'engagements', description='Endpoints for Engagements Management') | ||
| """Custom exception messages | ||
| """ | ||
|
|
||
| ENGAGEMENT_NOT_FOUND = 'Engagement was not found' | ||
|
|
||
|
|
||
| @cors_preflight('GET,OPTIONS') | ||
| @API.route('/<engagement_id>') | ||
|
|
@@ -53,9 +57,36 @@ def get(engagement_id): | |
| if engagement_record: | ||
| return engagement_record, HTTPStatus.OK | ||
|
|
||
| return 'Engagement was not found', HTTPStatus.INTERNAL_SERVER_ERROR | ||
| return ENGAGEMENT_NOT_FOUND, HTTPStatus.NOT_FOUND | ||
| except KeyError: | ||
| return 'Engagement was not found', HTTPStatus.INTERNAL_SERVER_ERROR | ||
| return ENGAGEMENT_NOT_FOUND, HTTPStatus.NOT_FOUND | ||
| except ValueError as err: | ||
| return str(err), HTTPStatus.INTERNAL_SERVER_ERROR | ||
|
|
||
|
|
||
| @cors_preflight('GET,OPTIONS') | ||
| @API.route('/slug/<slug>') | ||
| class EngagementBySlug(Resource): | ||
| """Resource for fetching a single engagement by slug.""" | ||
|
|
||
| @staticmethod | ||
| @cross_origin(origins=allowedorigins()) | ||
| @auth.optional | ||
| def get(slug): | ||
| """Fetch a single engagement matching the provided slug.""" | ||
| default_tenant = current_app.config.get( | ||
| 'DEFAULT_TENANT_SHORT_NAME', 'default') | ||
| tenant_short_name = request.headers.get('tenant-id', default_tenant) | ||
| tenant = TenantModel.find_by_short_name(tenant_short_name) | ||
| try: | ||
| engagement_record = EngagementService().get_engagement_by_slug(slug, tenant.id) | ||
|
|
||
| if engagement_record: | ||
| return engagement_record, HTTPStatus.OK | ||
|
|
||
| return ENGAGEMENT_NOT_FOUND, HTTPStatus.NOT_FOUND | ||
| except KeyError: | ||
| return ENGAGEMENT_NOT_FOUND, HTTPStatus.NOT_FOUND | ||
| except ValueError as err: | ||
| return str(err), HTTPStatus.INTERNAL_SERVER_ERROR | ||
|
|
||
|
|
@@ -74,7 +105,7 @@ def delete(engagement_id: int): | |
| EngagementService.delete(engagement_id) | ||
| return {'id': engagement_id}, HTTPStatus.OK | ||
| except KeyError as err: | ||
| return str(err), HTTPStatus.INTERNAL_SERVER_ERROR | ||
| return str(err), HTTPStatus.NOT_FOUND | ||
| except ValueError as err: | ||
| return str(err), HTTPStatus.BAD_REQUEST | ||
|
|
||
|
|
@@ -159,11 +190,11 @@ def post(): | |
| engagement_model = EngagementService().create_engagement(requestjson) | ||
| return engagement_schema.dump(engagement_model), HTTPStatus.OK | ||
| except KeyError as err: | ||
| return str(err), HTTPStatus.INTERNAL_SERVER_ERROR | ||
| return str(err), HTTPStatus.NOT_FOUND | ||
| except ValueError as err: | ||
| return str(err), HTTPStatus.INTERNAL_SERVER_ERROR | ||
| return str(err), HTTPStatus.NOT_FOUND | ||
| except ValidationError as err: | ||
| return str(err.messages), HTTPStatus.INTERNAL_SERVER_ERROR | ||
| return str(err.messages), HTTPStatus.BAD_REQUEST | ||
|
|
||
| @staticmethod | ||
| @cross_origin(origins=allowedorigins()) | ||
|
|
@@ -177,12 +208,14 @@ def patch(): | |
|
|
||
| engagement_schema = EngagementSchema() | ||
| payload = engagement_schema.load(requestjson, partial=True) | ||
| if not isinstance(payload, dict): | ||
| return 'Invalid engagement payload', HTTPStatus.BAD_REQUEST | ||
| engagement = EngagementService().edit_engagement(payload) | ||
|
|
||
| return engagement_schema.dump(engagement), HTTPStatus.OK | ||
| except KeyError as err: | ||
| return str(err), HTTPStatus.INTERNAL_SERVER_ERROR | ||
| return str(err), HTTPStatus.NOT_FOUND | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice, this error code seems more specific. |
||
| except ValueError as err: | ||
| return str(err), HTTPStatus.INTERNAL_SERVER_ERROR | ||
| return str(err), HTTPStatus.NOT_FOUND | ||
| except ValidationError as err: | ||
| return str(err.messages), HTTPStatus.BAD_REQUEST | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank goodness, this app has way too many tables right now IMO