DEP-265: Remove engagement slug table; merge into engagements by NatSquared · Pull Request #2876 · bcgov/dep-public

NatSquared · 2026-06-16T19:26:05Z

Issue #: 🎟️ DEP-265

Description of changes:

Feature Merged engagement slug into engagement requests
- Removed the EngagementSlug model, schema, and resource from the API, as they are no longer needed.
- Updated the Engagement model to include a slug field, which is now used to store the engagement slug directly in the engagement record.
- Engagements can now be fetched by slug, rather than fetching the slug then fetching the engagement by ID. This hastens the process of retrieving engagement data based on the slug.
- Engagements now return the slug in the engagement response, so that the front end can use it without needing to make a separate request to fetch the slug.
- Updated the front end to reflect these changes, including any API calls that previously referenced the EngagementSlug resource.

User Guide update ticket (if applicable):

- Yes, a user guide update ticket has been created. (Link to ticket)
- No, a user guide update ticket is not required.

Common component changes:

- Yes, I have updated CONTRIBUTING.md and the docblocks to document any changes to the common components.
- No, there are no changes to the common components.

codecov-commenter · 2026-06-16T19:30:35Z

Codecov Report

❌ Patch coverage is 79.59184% with 40 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.48%. Comparing base (5ae1815) to head (5fe5d9e).
⚠️ Report is 39 commits behind head on main.

Files with missing lines	Patch %	Lines
web/src/services/engagementService/index.ts	63.88%	13 Missing ⚠️
...rc/components/publicDashboard/DashboardContext.tsx	11.11%	8 Missing ⚠️
api/src/api/services/engagement_service.py	68.75%	5 Missing ⚠️
api/src/api/resources/engagement.py	87.87%	4 Missing ⚠️
api/src/api/services/authorization.py	90.90%	3 Missing ⚠️
api/src/api/services/submission_service.py	33.33%	2 Missing ⚠️
api/src/api/services/email_verification_service.py	92.30%	1 Missing ⚠️
...m/EngagementWidgets/Documents/CreateFolderForm.tsx	0.00%	1 Missing ⚠️
...m/EngagementWidgets/Documents/DocumentsContext.tsx	50.00%	1 Missing ⚠️
...ementWidgets/WhoIsListening/WhoIsListeningForm.tsx	0.00%	1 Missing ⚠️
... and 1 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2876      +/-   ##
==========================================
+ Coverage   73.37%   73.48%   +0.10%     
==========================================
  Files         521      516       -5     
  Lines       21066    20836     -230     
  Branches     1826     1816      -10     
==========================================
- Hits        15458    15311     -147     
+ Misses       5598     5515      -83     
  Partials       10       10

Flag	Coverage Δ
api	`84.05% <87.70%> (-0.17%)`	⬇️
web	`60.84% <66.21%> (+0.31%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
api/src/api/models/__init__.py	`100.00% <ø> (ø)`
api/src/api/models/engagement.py	`80.00% <100.00%> (+0.18%)`	⬆️
api/src/api/resources/__init__.py	`100.00% <100.00%> (ø)`
api/src/api/resources/engagement_members.py	`92.30% <100.00%> (ø)`
api/src/api/schemas/engagement.py	`96.36% <100.00%> (+0.06%)`	⬆️
...src/api/services/engagement_translation_service.py	`88.13% <100.00%> (-0.20%)`	⬇️
web/src/apiManager/endpoints/index.ts	`100.00% <ø> (ø)`
web/src/components/common/Input/TextInput.tsx	`100.00% <ø> (ø)`
...mponents/engagement/form/EngagementFormWrapper.tsx	`91.66% <100.00%> (-8.34%)`	⬇️
...ts/engagement/form/EngagementWidgets/Poll/Form.tsx	`69.60% <100.00%> (-0.25%)`	⬇️
... and 17 more

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

jareth-whitney

Excellent job with the DB simplification of the slugs. It's not glamorous work but it's very much needed, and it would be amazing if we could do more of this database optimization in the future. Nothing blocking, just check out the following:

Exclamation for type assertion flagged by Sonar Cube Cloud
Alembic automatic generation comments in migration
engagement_id defined as a string and as a number (probably intentional)
loaderData variable assignments (consider loaderData object without destructure)
Number.isNaN(Number()) could be followed by a > 0 check
Consider one-time type conversion to number for engagement ID instead of multiple conversions
Error concat, optional full colon

jareth-whitney · 2026-06-17T17:57:35Z

+                    existing_type=sa.String(length=256),
+                    nullable=False)
+    op.create_unique_constraint(
+        'uq_slug_per_tenant', 'engagement', ['slug', 'tenant_id'])


Nice, so a different tenant can use the same slug

jareth-whitney · 2026-06-17T17:58:17Z

+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###


You can probably get rid of these Alembic comments, unless it was all auto generated.

jareth-whitney · 2026-06-17T18:00:23Z

+API.add_namespace(ENGAGEMENT_METADATA_API,
+                  path='/engagements/<int:engagement_id>/metadata')
+API.add_namespace(ENGAGEMENT_MEMBERS_API,
+                  path='/engagements/<string:engagement_id>/members')


Just noticing that engagement_id is defined as a string or an integer for different endpoints, I'm guessing that's intentional.

jareth-whitney · 2026-06-17T18:01:54Z

            return engagement_schema.dump(engagement), HTTPStatus.OK
        except KeyError as err:
-            return str(err), HTTPStatus.INTERNAL_SERVER_ERROR
+            return str(err), HTTPStatus.NOT_FOUND


Nice, this error code seems more specific.

jareth-whitney · 2026-06-17T18:04:01Z

    name = fields.Str(data_key='name', required=True, validate=validate.Length(
        min=1, error='Name cannot be blank'))
+    slug = fields.Str(data_key='slug', required=True, validate=validate.Length(
+        min=1, error='Slug cannot be blank'))


Should we return a validation error if the slug is too long as well?

jareth-whitney · 2026-06-17T18:38:25Z

            return;
        }
-        if (isNaN(Number(engagementId))) {
+        if (Number.isNaN(Number(engagementId))) {


You might want to add a second check for greater than 0 here, as some values like an empty string are converted to 0 with Number(), and that would still return valid.

jareth-whitney · 2026-06-17T18:41:24Z

            navigate(getPath(ROUTES.PUBLIC_NOT_FOUND));
            return;
        }
+        const engagementPromise = engagementId ? getEngagement(engagementId) : getEngagementBySlug(slug!);


It looks like Sonar wants you to remove the exclamation here, but that's a neat trick.

jareth-whitney · 2026-06-17T18:44:00Z

+    const getEngPromise = () => {
+        if (urlSlug) {
+            return getEngagementBySlug(urlSlug);
+        } else if (engagementId && !Number.isNaN(Number(engagementId))) {


Optionally, you could convert the value to a number once at the top of the file.

jareth-whitney · 2026-06-17T18:45:04Z

-    if (!engagementId || isNaN(Number(engagementId))) {
-        return Promise.reject('Invalid Engagement Id ' + engagementId);
+    if (!engagementId || Number.isNaN(Number(engagementId))) {
+        throw new Error('Invalid Engagement Id ' + engagementId);


I know this is existing code but it could be good to add a full colon:

Invalid engagement ID: 34

jareth-whitney · 2026-06-17T18:46:05Z

+
+- **Feature** Merged engagement slug into engagement requests [🎟️ DEP-265](https://citz-gdx.atlassian.net/browse/DEP-265)
+  - Removed the EngagementSlug model, schema, and resource from the API, as they are no longer needed.
+  - Updated the Engagement model to include a `slug` field, which is now used to store the engagement slug directly in the engagement record.


Thank goodness, this app has way too many tables right now IMO

sonarqubecloud · 2026-06-18T21:48:46Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

DEP-265: Remove engagement slug table; merge into engagements

4674f0a

NatSquared added 4 commits June 16, 2026 12:46

Fix Sonarcloud issues

5274f9c

Fix linter issue

4a8db7d

Update CHANGELOG

3b63c35

Fix widget unit tests

adda3e5

NatSquared marked this pull request as ready for review June 16, 2026 20:35

NatSquared requested a review from jareth-whitney June 16, 2026 20:37

Replace ActionContext with router loaders

f82259a

jareth-whitney approved these changes Jun 17, 2026

View reviewed changes

NatSquared added 3 commits June 17, 2026 12:51

Apply fixes from PR suggestions

c4bbaf8

Add missing unit tests

0f94440

Fix Sonar issues; add getEngagement to test

5fe5d9e

NatSquared merged commit 793545d into main Jun 18, 2026
16 checks passed



		def upgrade():
		# ### commands auto generated by Alembic - please adjust! ###

Uh oh!

Conversation

NatSquared commented Jun 16, 2026

Uh oh!

codecov-commenter commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jareth-whitney left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqubecloud Bot commented Jun 18, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Jun 16, 2026 •

edited

Loading