fix: multiple twenty orm issues & show an example of use #5439

magrinj · 2024-05-16T13:50:54Z

This PR is fixing some issues and adding enhancement in TwentyORM:

Composite fields in nested relations are not formatted properly
Passing operators like Any in where condition is breaking the query
Ability to auto load workspace-entities based on a regex path

I've also introduced an example of use for CalendarEventService:

https://github.com/twentyhq/twenty/pull/5439/files#diff-3a7dffc0dea57345d10e70c648e911f98fe237248bcea124dafa9c8deb1db748R15

github-actions · 2024-05-16T13:51:52Z

TODOs/FIXMEs:

// TODO: Align return type with the entities to avoid mapping: packages/twenty-server/src/engine/core-modules/calendar/timeline-calendar-event.service.ts

Generated by 🚫 dangerJS against a445d24

magrinj · 2024-05-16T13:52:03Z

packages/twenty-server/src/engine/core-modules/calendar/timeline-calendar-event.service.ts

    personIds: string[],
    page = 1,
    pageSize: number = TIMELINE_CALENDAR_EVENTS_DEFAULT_PAGE_SIZE,
  ): Promise<TimelineCalendarEventsWithTotal> {
    const offset = (page - 1) * pageSize;

-    const dataSourceSchema =
-      this.workspaceDataSourceService.getSchemaName(workspaceId);
+    const calendarEventIds = await this.calendarEventRepository.find({


We need to do two separate calls because TypeORM by default is filtering the where condition in the relations.

magrinj · 2024-05-16T13:52:54Z

packages/twenty-server/src/engine/core-modules/calendar/timeline-calendar-event.service.ts

-          displayName,
-          avatarUrl,
+    const timelineCalendarEvents = orderedEvents.map((event) => {
+      const participants = event.calendarEventParticipants.map(


Maybe we should rewrite the output, and use something that is more like the database entities ?

Yes the heavy part is done here instead of the FE but that's usually not how we do it in the rest of the app I think so we could re-arrange that

magrinj · 2024-05-16T13:53:29Z

packages/twenty-server/src/engine/core-modules/core-engine.module.ts


 import { AnalyticsModule } from './analytics/analytics.module';
 import { FileModule } from './file/file.module';
 import { ClientConfigModule } from './client-config/client-config.module';

 @Module({
  imports: [
+    TwentyORMModule.register({
+      workspaceEntities: ['dist/src/**/*.workspace-entity{.ts,.js}'],


workspaceEntities can now be auto-loaded based on a path regex

magrinj · 2024-05-16T13:55:06Z

packages/twenty-server/src/engine/twenty-orm/repository/workspace.repository.ts

          newData[key] = this.formatResult(value);
        } else {
          newData[key] = value;
        }
        continue;
      }

+      if (relationMetadataArgs) {


We can't stick to just one level of formatting result, as we'll often need to use relations.
So we're now recursively formatting them based on the relationMetadataArgs.
We should be careful with that, and properly test performance behaviour.
In my test I didn't get real difference before and after.

magrinj · 2024-05-16T13:55:45Z

packages/twenty-server/src/engine/twenty-orm/twenty-orm-core.module.ts

+import { importClassesFromDirectories } from 'typeorm/util/DirectoryExportedClassesLoader';
+import { Logger as TypeORMLogger } from 'typeorm/logger/Logger';


Not really pretty, but I think it's better than just rewriting this functions internally.

Weiko · 2024-05-17T08:27:55Z

packages/twenty-server/src/engine/core-modules/calendar/timeline-calendar-event.service.ts


-    if (!calendarEvents) {
+    if (ids.length <= 0) {


How could this one be negative?

This shouldn't happen, it's just an old habit

Weiko · 2024-05-17T08:29:37Z

packages/twenty-server/src/engine/core-modules/calendar/timeline-calendar-event.service.ts

-          displayName,
-          avatarUrl,
+    const timelineCalendarEvents = orderedEvents.map((event) => {
+      const participants = event.calendarEventParticipants.map(


Yes the heavy part is done here instead of the FE but that's usually not how we do it in the rest of the app I think so we could re-arrange that

Weiko · 2024-05-17T08:30:21Z

packages/twenty-server/src/engine/core-modules/calendar/timeline-calendar-event.service.ts

-    const timelineCalendarEvents = calendarEvents.map((event) => {
-      const participants = calendarEventParticipantsByEventId[event.id] || [];
+      const visibility = event.calendarChannelEventAssociations.some(
+        (association) => association.calendarChannel.visibility === 'METADATA',


Does it not return a TimelineCalendarEventVisibility? Can't we compare with TimelineCalendarEventVisibility.METADATA?

Weiko · 2024-05-17T08:30:56Z

packages/twenty-server/src/engine/core-modules/calendar/timeline-calendar-event.service.ts


-    if (!personIds) {
+    if (personIds.length <= 0) {


Weiko · 2024-05-17T08:32:53Z

...engine/metadata-modules/workspace-migration/utils/field-metadata-type-to-column-type.util.ts

@@ -26,7 +26,7 @@ export const fieldMetadataTypeToColumnType = <Type extends FieldMetadataType>(
    case FieldMetadataType.DATE_TIME:
      return 'timestamptz';
    case FieldMetadataType.DATE:
-      return 'date';
+      return 'datetime';


Is this intended for this PR? Why this change?

It's something I forgot when debugging sorry

Weiko · 2024-05-17T08:36:33Z

packages/twenty-server/src/engine/twenty-orm/twenty-orm-core.module.ts

+    const [entityClassesOrSchemas, entityDirectories] = splitClassesAndStrings(
+      workspaceEntities || [],
+    );
+    const importedEntities = await importClassesFromDirectories(


Great that we can reuse this typeorm function!

Weiko · 2024-05-17T08:38:07Z

packages/twenty-server/src/engine/twenty-orm/twenty-orm-core.module.ts

+      ...(importedEntities as Type<BaseWorkspaceEntity>[]),
+    ];
+
+    return entities.filter(


Maybe we can handle this from the regex directly so this way it's not trying to import the class earlier for nothing?

charlesBochet

Feedbacks:
I would like to revisit the naming: WorkspaceEntity ==> ObjectEntity
Also, I don't understand the date => datetime change for FieldMetadata type Date

Tested and it works well congrats! I'm going to merge it, let's prioritize the date => datetime discussion tomorrow

commit bb2d74f Merge: b9c83a4 a981344 Author: Shubham Pawar <[email protected]> Date: Tue May 21 10:37:10 2024 +0530 Merge branch 'main' of https://github.com/synapsenet-arena/lead360 commit a981344 Author: rostaklein <[email protected]> Date: Mon May 20 17:29:35 2024 +0200 feat: fetch and parse full gmail message (twentyhq#5160) first part of twentyhq#4108 related PR twentyhq#5081 --------- Co-authored-by: Charles Bochet <[email protected]> commit b5d3396 Author: bosiraphael <[email protected]> Date: Mon May 20 17:19:21 2024 +0200 5477 - Introduce syncsubstatus in db to refactor gmail sync behavior (twentyhq#5479) Closes twentyhq#5477 commit 737fffe Author: Indrakant D <[email protected]> Date: Mon May 20 20:29:01 2024 +0530 Fix: danger font color values & acc. to design specs (twentyhq#5344) fixes: twentyhq#5325 changes done (commits in order): 1. **Fixed fontLight & fontDark 'danger' color as per design spec**: changed theme.font.color.danger to match the disabled color theme (for light and dark) as followed by the BorderDark and BorderLight. Use the updated colors for Buttons 2. **Replace theme.font.color.danger with theme.color.red (5 changed files)**: Since `theme.font.color.danger` has now been updated to contain the disabled button color values, we use the `theme.color.red` color in all the places using `theme.font.color.danger` as it contains same value that was used to be of `theme.font.color.danger` before. 3. **fixed hover color of StyledConfirmationButton in ConfirmationModal**: issue can be seen when going to /settings/workspace and trying to hover on delete the workspace button in dark mode. fixed with this commit. **Important Note**: The files `/twenty-front/src/modules/ui/theme/constants/FontLight.ts` and `/twenty-front/src/modules/ui/theme/constants/FontDark.ts` **are of no use** as theme for the entire 'twenty-front' and 'twenty-chrome-extension' packages use the same files from '@/ui/theme' (twenty-ui package) dark mode : <img width="987" alt="Screenshot 2024-05-09 at 9 14 35 PM" src="https://github.com/twentyhq/twenty/assets/60315832/75fe3972-0e8a-41f6-90a1-09bfcd013e72"> when disabled: <img width="1098" alt="Screenshot 2024-05-09 at 9 13 46 PM" src="https://github.com/twentyhq/twenty/assets/60315832/5caab8b5-47ba-43e5-90cd-a41a1f690ca0"> on hover: <img width="1052" alt="Screenshot 2024-05-09 at 9 14 05 PM" src="https://github.com/twentyhq/twenty/assets/60315832/58de3df6-ed77-4aad-84fc-67b01154b493"> <br> <br> light mode (when disabled): <img width="918" alt="Screenshot 2024-05-09 at 9 13 14 PM" src="https://github.com/twentyhq/twenty/assets/60315832/18228783-d6c7-44a6-9fce-00053bb35ef2"> on hover: <img width="983" alt="Screenshot 2024-05-09 at 9 14 18 PM" src="https://github.com/twentyhq/twenty/assets/60315832/6df99f12-5767-4136-80c9-5d8883ac8e00"> --------- Co-authored-by: Félix Malfait <[email protected]> commit 4d479ee Author: Thomas Trompette <[email protected]> Date: Mon May 20 16:37:35 2024 +0200 Remove relations for remotes (twentyhq#5455) For remotes, we will only create the foreign key, without the relation metadata. Expected behavior will be: - possible to create an activity. But the remote object will not be displayed in the relations of the activity - the remote objects should not be available in the search for relations Also switched the number settings to an enum, since we now have to handle `BigInt` case. --------- Co-authored-by: Thomas Trompette <[email protected]> commit b098027 Author: Weiko <[email protected]> Date: Mon May 20 15:53:13 2024 +0200 Fix graphql prep query (twentyhq#5478) commit 88f5eb6 Author: martmull <[email protected]> Date: Mon May 20 12:11:38 2024 +0200 4689 multi workspace i should be able to accept an invite if im already logged in (twentyhq#5454) - split signInUp to separate Invitation from signInUp - update redirection logic - add a resolver for userWorkspace - add a mutation to add a user to a workspace - authorize /invite/hash while loggedIn - add a button to join a workspace ### Base functionnality https://github.com/twentyhq/twenty/assets/29927851/a1075a4e-a2af-4184-aa3e-e163711277a1 ### Error handling https://github.com/twentyhq/twenty/assets/29927851/1bdd78ce-933a-4860-a87a-3f1f7bda389e commit 1ceeb68 Author: ktang520 <[email protected]> Date: Mon May 20 02:31:39 2024 -0700 Changed record chip functionality from onClick to anchor tag (twentyhq#5462) [twentyhq#4422](twentyhq#4422) Demo: https://github.com/twentyhq/twenty/assets/155670906/f8027ab2-c579-45f7-9f08-f4441a346ae7 Within the demo, we show the various areas in which the Command/CTRL + Click functionality works. The table cells within the People and Companies tab open within both the current tab and new tab due to unchanged functionality within RecordTableCell. We did this to ensure we could get a PR within by the end of the week. In this commit, we ONLY edited EntityChip.tsx. We did this by: - Removing useNavigate() and handleLinkClick/onClick functionality - Wrapping InnerEntityChip in an anchor tag This allowed for Command/CTRL + Click functionality to work. Clickable left cells on tables, left side menu, and data model navigation files/areas DID NOT get updated. --------- Co-authored-by: Félix Malfait <[email protected]> commit 8b5f79d Author: Jérémy M <[email protected]> Date: Mon May 20 11:01:47 2024 +0200 fix: multiple twenty orm issues & show an example of use (twentyhq#5439) This PR is fixing some issues and adding enhancement in TwentyORM: - [x] Composite fields in nested relations are not formatted properly - [x] Passing operators like `Any` in `where` condition is breaking the query - [x] Ability to auto load workspace-entities based on a regex path I've also introduced an example of use for `CalendarEventService`: https://github.com/twentyhq/twenty/pull/5439/files#diff-3a7dffc0dea57345d10e70c648e911f98fe237248bcea124dafa9c8deb1db748R15 commit 81e8f49 Author: H0onnn <[email protected]> Date: Mon May 20 00:26:29 2024 +0900 Feat : Change title color of release page in dark mode (twentyhq#5467) ## Issue - close twentyhq#5459 ## Work Detail Change title color of release page in dark mode. I worked using the useColorScheme and useSystemColorSheme hooks, but if there is a better way, please recommend it. ## Before <img width="606" alt="image" src="https://github.com/twentyhq/twenty/assets/116232939/f5c05360-f1d5-4701-b17d-e3e8a1db65fa"> ## After <img width="565" alt="image" src="https://github.com/twentyhq/twenty/assets/116232939/5f9460d3-db62-461f-b7c2-659a4b687ba9"> --------- Co-authored-by: Félix Malfait <[email protected]> commit 66637a3 Author: Weiko <[email protected]> Date: Sat May 18 08:00:00 2024 +0200 Add more details to mutation limit exception message and fix update many query (twentyhq#5460) ## Context Since we rely on PgGraphql to query the DB, we have to map its errors to more comprehensible errors before sending them back to the FE. This has already been done for unicity constraint and mutation maximum records but for the last one the message wasn't clear enough. This PR introduces a new pgGraphqlConfig param to the util to pass down the 'atMost' config that we are actually overwriting with an 'MUTATION_MAXIMUM_RECORD_AFFECTED' env variable. See how atMost works in this doc (https://supabase.github.io/pg_graphql/api/#delete) Also adding the same message for the update since this mutation is also affected. Create is not though. Lastly, this PR introduces a fix on the updateMany. Since the current FE is not using updateMany, this was missed for a few weeks but a regression has been introduced when we started checking if the id is a valid UUID however for updateMany this was checking the data object instead of the filter object. Actually, the data object should never contain id because it wouldn't make sense to allow the update of the id and even more for multiple records since the id should be unique. ## Test locally with MUTATION_MAXIMUM_RECORD_AFFECTED=5 <img width="1408" alt="Screenshot 2024-05-18 at 02 11 59" src="https://github.com/twentyhq/twenty/assets/1834158/06bf25ce-4a44-4851-8456-aed7689bb33e"> <img width="1250" alt="Screenshot 2024-05-18 at 02 12 10" src="https://github.com/twentyhq/twenty/assets/1834158/06fc4329-147b-4bb4-9223-c3bce340a8d2"> <img width="1222" alt="Screenshot 2024-05-18 at 02 12 36" src="https://github.com/twentyhq/twenty/assets/1834158/0674546e-73e2-4e5c-918f-9825f2ee5967"> <img width="1228" alt="Screenshot 2024-05-18 at 02 13 01" src="https://github.com/twentyhq/twenty/assets/1834158/f50df435-1fd4-45df-a953-8fefa8f36e75"> <img width="1174" alt="Screenshot 2024-05-18 at 02 13 09" src="https://github.com/twentyhq/twenty/assets/1834158/707b9300-2779-43df-8177-9658b8965b49"> <img width="1393" alt="Screenshot 2024-05-18 at 02 19 11" src="https://github.com/twentyhq/twenty/assets/1834158/2cd167b6-1261-4914-a4db-36f792d810c0"> commit 0e525ca Author: gitstart-app[bot] <57568882+gitstart-app[bot]@users.noreply.github.com> Date: Fri May 17 16:36:28 2024 +0200 Implement <ScrollRestoration /> (twentyhq#5086) ### Description Implement <ScrollRestoration /> ### Refs [https://github.com/twentyhq/twenty/issues/4357](https://github.com/twentyhq/twenty/issues/4183) ### Demo https://github.com/twentyhq/twenty/assets/140154534/321242e1-4751-4204-8c86-e9b921c1733e Fixes twentyhq#4357 --------- Co-authored-by: gitstart-twenty <[email protected]> Co-authored-by: Lucas Bordeau <[email protected]> Co-authored-by: v1b3m <[email protected]> Co-authored-by: RubensRafael <[email protected]> commit 992602b Author: Thaïs <[email protected]> Date: Fri May 17 16:05:31 2024 +0200 fix: fix storybook build cache not being used by tests in CI (twentyhq#5451) TL;DR: - removed `--configuration={args.scope}` from `storybook:static:test` for the `storybook:static` part, as it was making `front-sb-test` jobs in CI not reuse the cache from the `front-sb-build` job and re-build storybook every time. - replaced it with a new `test` configuration which optimizes storybook build for tests and builds storybook 2x faster. ## Fix storybook:build cache usage in CI `storybook:static:test` executes two scripts in parallel: 1. `storybook:static`, which depends on `storybook:build` 1.a. it builds storybook first with `storybook:build`, the output directory is `storybook-static`. 1.b. then it launches an `http-server`, using what has been built in `storybook-static` 2. `storybook:test` to execute tests (needs the storybook http-server to be running) When passing `--configuration=pages` or `--configuration=modules` to `storybook:static` from step 1, those configurations are passed to the `storybook:build` script from step 1.a as well. But for Nx `storybook:build` and `storybook:build --configuration=pages` (or `modules`) are not the same command, therefore one does not reuse the cache of the other because they could output completely different things. As `front-sb-test` jobs are passing `--configuration={args.scope}` to `storybook:static`, the cache of the previously executed `storybook:build` (from `front-sb-build`) is not reused and therefore each job re-builds Storybook with its own scope, which increases CI time. ### Solution - Removed scope configurations from `storybook:static` and `storybook:build` scripts to avoid confusion. - `storybook:test` and `storybook:dev` can keep scope configurations as they can be useful and this doesn't impact storybook build cache in CI. ### Improve Storybook build time for testing Added the `test` configuration to `storybook:build` and `storybook:static` which makes Storybook build time 2x faster. It disables addons that slow down build time and are not used in tests. commit 36e5411 Author: Thomas Trompette <[email protected]> Date: Fri May 17 10:38:17 2024 +0200 Enable remotes with existing name (twentyhq#5433) - Check if a table with the same name already exists - If yes, add a number suffix, and check again Co-authored-by: Thomas Trompette <[email protected]> commit 58f8c31 Author: Félix Malfait <[email protected]> Date: Fri May 17 09:10:59 2024 +0200 Fixes typo in docs twentyhq#5076 (twentyhq#5450) Micro fix Fixes twentyhq#5076

This PR is fixing some issues and adding enhancement in TwentyORM: - [x] Composite fields in nested relations are not formatted properly - [x] Passing operators like `Any` in `where` condition is breaking the query - [x] Ability to auto load workspace-entities based on a regex path I've also introduced an example of use for `CalendarEventService`: https://github.com/twentyhq/twenty/pull/5439/files#diff-3a7dffc0dea57345d10e70c648e911f98fe237248bcea124dafa9c8deb1db748R15

fix: multiple twenty orm issues & show an example of use

e75adb8

magrinj commented May 16, 2024

View reviewed changes

fix: merge

a445d24

magrinj requested a review from Weiko May 16, 2024 14:15

magrinj marked this pull request as ready for review May 16, 2024 14:15

magrinj added the -PR: awaiting review label May 16, 2024

charlesBochet assigned Weiko and charlesBochet May 17, 2024

Weiko reviewed May 17, 2024

View reviewed changes

charlesBochet approved these changes May 20, 2024

View reviewed changes

charlesBochet merged commit 8b5f79d into main May 20, 2024
5 checks passed

charlesBochet deleted the fix/twenty-orm branch May 20, 2024 09:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: multiple twenty orm issues & show an example of use #5439

fix: multiple twenty orm issues & show an example of use #5439

magrinj commented May 16, 2024 •

edited

Loading

github-actions bot commented May 16, 2024 •

edited

Loading

magrinj May 16, 2024

magrinj May 16, 2024

Weiko May 17, 2024

magrinj May 16, 2024

magrinj May 16, 2024

magrinj May 16, 2024

Weiko May 17, 2024

magrinj May 21, 2024

Weiko May 17, 2024

Weiko May 17, 2024

Weiko May 17, 2024

Weiko May 17, 2024

magrinj May 21, 2024

Weiko May 17, 2024

Weiko May 17, 2024

charlesBochet left a comment

		import { importClassesFromDirectories } from 'typeorm/util/DirectoryExportedClassesLoader';
		import { Logger as TypeORMLogger } from 'typeorm/logger/Logger';

fix: multiple twenty orm issues & show an example of use #5439

fix: multiple twenty orm issues & show an example of use #5439

Conversation

magrinj commented May 16, 2024 • edited Loading

github-actions bot commented May 16, 2024 • edited Loading

TODOs/FIXMEs:

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

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

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

charlesBochet left a comment

Choose a reason for hiding this comment

magrinj commented May 16, 2024 •

edited

Loading

github-actions bot commented May 16, 2024 •

edited

Loading