feat: read from content_preference source member #2451

capJavert · 2024-11-15T16:46:16Z

read from ContentPreferenceSource instead of SourceMember
test updates
- lots of changes, good thing is that none of them really failed due to logic but just difference in test setup (content preference vs source member fixtures)
- adjust tests to use ContentPreferenceSource instead of SourceMember
- some tests also include SourceMember due to backward compatibility (we still write to SourceMember as well)
triggers left as is until we completely remove SourceMember
all queries read from ContentPreference but fields return the same source member fields so no changes needed for frontend (see graphorm/index.ts fields changes)
content_preference table is now used for cdc instead of source_member (debezium config updated)

idoshamun · 2024-11-20T09:20:16Z

src/common/feedGenerator.ts

+          'hide',
+        )
+        .from(ContentPreference, 't')
+        .innerJoin(


anyway we can get rid of this join? i also think that somewhere we already do this join so we can reuse it

Same issue as #2451 (comment)

idoshamun · 2024-11-20T09:22:51Z

src/schema/sources.ts

          } else if (
            typeof graphorm.mappings?.SourceMember.fields?.roleRank.select ===
            'string'
          ) {
            queryBuilder = queryBuilder.andWhere(
              `${graphorm.mappings.SourceMember.fields.roleRank.select} >= 0`,
            );
+          } else if (
+            typeof graphorm.mappings!.SourceMember!.fields!.roleRank.select ===


What is this addition?

Field is now a function in graphorm/index, before it was a string so I had to adjust it. See https://github.com/dailydotdev/daily-api/pull/2451/files#diff-928042d056cac827c70794ef59920facfe32f158ebf447ff7d2826fcad5195aeR541-R542

idoshamun · 2024-11-20T09:24:16Z

src/workers/cdc/primary.ts

+  const sourceId =
+    data.payload.after!.sourceId || data.payload.before!.sourceId;
+
+  const source = await con


Maybe we should consider storing a flag that says on content preference that this is a squad? It will make fetching the source redundant and the content preference object will be complete

I thought about that, initially ContentPreference had a different type for squad all together but I opted to make it source because in more places we use them interchangeably.

Adding another flag for this differentiation would solve it, but then its another field that has to be synced and set here and on source.type itself if type of source changes (it does not happen often I think, or never).

You think this would be expensive if we don't add the flag? I added additional check so its only queried for source type.

I think it's a downgrade from what we had and might make it unclear for other devs that it needs to be queried

Thinking, should we have gone with a route to have different type for squad content_preference then.

Since it's water under the bridge now, let's proceed as is then. We should just be clear about it and pay attention

idoshamun

Massive work @capJavert, way to go!!!

feat: do not show status blocked in member lists feat: do not send notifications for status blocked members feat: still show member is part of source if status was blocked (since new UI is missing this is temp nuances)

capJavert

I added some missed stuff to not show blocked squads i member lists and sidebar and also to avoid sending them notifications.

capJavert · 2024-11-28T15:30:41Z

src/workers/cdc/primary.ts

+  const sourceId =
+    data.payload.after!.sourceId || data.payload.before!.sourceId;
+
+  const source = await con


I thought about that, initially ContentPreference had a different type for squad all together but I opted to make it source because in more places we use them interchangeably.

Adding another flag for this differentiation would solve it, but then its another field that has to be synced and set here and on source.type itself if type of source changes (it does not happen often I think, or never).

You think this would be expensive if we don't add the flag? I added additional check so its only queried for source type.

capJavert · 2024-11-28T15:43:06Z

src/schema/sources.ts

          } else if (
            typeof graphorm.mappings?.SourceMember.fields?.roleRank.select ===
            'string'
          ) {
            queryBuilder = queryBuilder.andWhere(
              `${graphorm.mappings.SourceMember.fields.roleRank.select} >= 0`,
            );
+          } else if (
+            typeof graphorm.mappings!.SourceMember!.fields!.roleRank.select ===


Field is now a function in graphorm/index, before it was a string so I had to adjust it. See https://github.com/dailydotdev/daily-api/pull/2451/files#diff-928042d056cac827c70794ef59920facfe32f158ebf447ff7d2826fcad5195aeR541-R542

capJavert · 2024-11-28T16:04:25Z

src/common/feedGenerator.ts

+          'hide',
+        )
+        .from(ContentPreference, 't')
+        .innerJoin(


Same issue as #2451 (comment)

capJavert · 2024-11-28T17:09:15Z

__tests__/workers/userUpdatedPlusSubscription.ts

+    const sourceMember = await con
+      .getRepository(ContentPreferenceSource)
+      .findOneBy({
+        referenceId: PLUS_MEMBER_SQUAD_ID,
+        userId: newBase.id,
+        status: Not(ContentPreferenceStatus.Blocked),
+      });


@rebelchris if you can sanity check test changes for plus in this file

functional change is only this https://github.com/dailydotdev/daily-api/pull/2451/files#diff-6f8bfa9682abac26b9a01cf5c808aef211169a25fec49c837320349bb368c4e3R43-R47

feat: read from content_preference source member

6ab2ed4

capJavert self-assigned this Nov 15, 2024

capJavert and others added 7 commits November 15, 2024 17:52

fix: raw query when selecting

fdcb7f3

feat: update tests

e47cda0

feat: notifications tests

d045cdf

feat: post tests

c10a83d

feat: source tests

4878e61

feat: debezium config

c4b7620

Merge branch 'main' into AS-681-content-preference-source-member

d6a758b

capJavert marked this pull request as ready for review November 18, 2024 14:19

capJavert requested a review from a team as a code owner November 18, 2024 14:19

capJavert requested review from rebelchris, omBratteng, ilasw, sshanzel, nensidosari, AmarTrebinjac and idoshamun November 18, 2024 14:19

idoshamun reviewed Nov 20, 2024

View reviewed changes

capJavert added 2 commits November 28, 2024 15:24

Merge branch 'main' into AS-681-content-preference-source-member

d8c1e8b

feat: adjust based on blocked status

9de27e1

feat: do not show status blocked in member lists feat: do not send notifications for status blocked members feat: still show member is part of source if status was blocked (since new UI is missing this is temp nuances)

capJavert commented Nov 28, 2024

View reviewed changes

refactor: plus member to content preference

9bb570c

capJavert requested a review from idoshamun November 28, 2024 17:08

capJavert commented Nov 28, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: read from content_preference source member #2451

feat: read from content_preference source member #2451

capJavert commented Nov 15, 2024 •

edited

Loading

idoshamun Nov 20, 2024

capJavert Nov 28, 2024

idoshamun Nov 20, 2024

capJavert Nov 28, 2024

idoshamun Nov 20, 2024

capJavert Nov 28, 2024 •

edited

Loading

idoshamun Nov 29, 2024

capJavert Nov 29, 2024

idoshamun Dec 1, 2024

idoshamun left a comment

capJavert left a comment

capJavert Nov 28, 2024 •

edited

Loading

capJavert Nov 28, 2024

capJavert Nov 28, 2024

capJavert Nov 28, 2024

capJavert Nov 28, 2024 •

edited

Loading

feat: read from content_preference source member #2451

Are you sure you want to change the base?

feat: read from content_preference source member #2451

Conversation

capJavert commented Nov 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

capJavert Nov 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

idoshamun left a comment

Choose a reason for hiding this comment

capJavert left a comment

Choose a reason for hiding this comment

capJavert Nov 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

capJavert Nov 28, 2024 • edited Loading

Choose a reason for hiding this comment

capJavert commented Nov 15, 2024 •

edited

Loading

capJavert Nov 28, 2024 •

edited

Loading

capJavert Nov 28, 2024 •

edited

Loading

capJavert Nov 28, 2024 •

edited

Loading