WIP : feed migrations to support multi-domain #22601
Conversation
sonika-shah
added
safe to test
There was a problem hiding this comment.
Pull Request Overview
This PR addresses database migrations and configuration updates to support multi-domain functionality. The changes migrate from single domain support to multi-domain arrays in the thread_entity table and update activity feed configurations accordingly.
- Migrate thread_entity table from single
domainto multi-domaindomainsarray - Update activity feed event configurations to track
domainsinstead ofdomain - Add generated columns for efficient domain querying
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| ActivityFeedEvents.json | Updates event subscription conditions to track domains field instead of domain |
| postgres/schemaChanges.sql | Migrates thread_entity table data and schema for PostgreSQL multi-domain support |
| mysql/schemaChanges.sql | Migrates thread_entity table data and schema for MySQL multi-domain support |
| to_jsonb(ARRAY[json->'domain']) | ||
| ) | ||
| WHERE json ? 'domain' AND json->'domain' IS NOT NULL; |
There was a problem hiding this comment.
The migration will create an array containing a null value when json->'domain' is null, despite the WHERE condition checking for NOT NULL. This could result in domains arrays like [null]. Consider using CASE to handle null values or adjust the WHERE condition to be more restrictive.
| to_jsonb(ARRAY[json->'domain']) | |
| ) | |
| WHERE json ? 'domain' AND json->'domain' IS NOT NULL; | |
| CASE | |
| WHEN json->'domain' IS NOT NULL AND json->>'domain' IS DISTINCT FROM 'null' | |
| THEN to_jsonb(ARRAY[json->'domain']) | |
| ELSE to_jsonb(ARRAY[]::text[]) | |
| END | |
| ) | |
| WHERE json ? 'domain' AND json->>'domain' IS DISTINCT FROM 'null'; |
Copilot uses AI. Check for mistakes.
| SET json = jsonb_set( | ||
| json #- '{feedInfo,entitySpecificInfo,entity,domain}', | ||
| '{feedInfo,entitySpecificInfo,entity,domains}', | ||
| to_jsonb(ARRAY[json#>'{feedInfo,entitySpecificInfo,entity,domain}']) |
There was a problem hiding this comment.
Similar to the previous migration, this could create an array with null values when the nested domain is null. The WHERE condition checks for NOT NULL but the array creation doesn't handle the null case properly.
| to_jsonb(ARRAY[json#>'{feedInfo,entitySpecificInfo,entity,domain}']) | |
| to_jsonb(CASE | |
| WHEN json#>'{feedInfo,entitySpecificInfo,entity,domain}' IS NULL THEN ARRAY[]::jsonb[] | |
| ELSE ARRAY[json#>'{feedInfo,entitySpecificInfo,entity,domain}'] | |
| END) |
Copilot uses AI. Check for mistakes.
| SET json = JSON_SET( | ||
| JSON_REMOVE(json, '$.domain'), | ||
| '$.domains', | ||
| JSON_ARRAY(JSON_EXTRACT(json, '$.domain')) |
There was a problem hiding this comment.
The migration could create arrays containing null values when JSON_EXTRACT returns null, despite the WHERE condition. MySQL's JSON_ARRAY will include null values in the array. Consider using CASE to handle null extractions properly.
| JSON_ARRAY(JSON_EXTRACT(json, '$.domain')) | |
| JSON_ARRAY( | |
| CASE | |
| WHEN JSON_EXTRACT(json, '$.domain') IS NOT NULL | |
| THEN JSON_EXTRACT(json, '$.domain') | |
| ELSE NULL | |
| END | |
| ) |
Copilot uses AI. Check for mistakes.
| JSON_ARRAY(JSON_EXTRACT(json, '$.feedInfo.entitySpecificInfo.entity.domain')) | ||
| ) |
There was a problem hiding this comment.
Similar null handling issue as above - JSON_ARRAY will include null values even when the WHERE condition attempts to filter them out. This could result in domains arrays containing null elements.
| JSON_ARRAY(JSON_EXTRACT(json, '$.feedInfo.entitySpecificInfo.entity.domain')) | |
| ) | |
| JSON_ARRAY( | |
| IF( | |
| JSON_EXTRACT(json, '$.feedInfo.entitySpecificInfo.entity.domain') IS NOT NULL, | |
| JSON_EXTRACT(json, '$.feedInfo.entitySpecificInfo.entity.domain'), | |
| NULL | |
| ) | |
| ) |
Copilot uses AI. Check for mistakes.
… list<uuid> rather than list<entityRef>
|
TypeScript types have been updated based on the JSON schema changes in the PR |
Jest test CoverageUI tests summary
|
|
|
Describe your changes:
Fixes
Type of change:
Checklist:
Fixes <issue-number>: <short explanation>