Commit Graph
48 Commits
Author SHA1 Message Date
Paul Rastoin 5327febcbe [REFACTOR] remove isRemote assertion from hasFieldPosition util (#10519)
Following this discussion
https://github.com/twentyhq/twenty/pull/10510#discussion_r1971845556
with @ijreilly
2025-02-26 18:28:55 +01:00
Paul Rastoin ea0d3c605f [FIX] Optimistically compute position only for objectMetadataItem that has the field (#10510)
# Introduction
In this PR https://github.com/twentyhq/twenty/pull/10493 introduced a
regression on optimistic cache for record creation, by expecting a
`position` `fieldMetadataItem` on every `ObjectMetadataItem` which is a
wrong assertion
Some `Tasks` and `ApiKeys` do not have one

## Fix
Dynamically compute optimistic record input position depending on
current `ObjectMetadataItem`

## Refactor
Refactored a failing test following [jest
each](https://jestjs.io/docs/api#describeeachtablename-fn-timeout)
pattern to avoid error prone duplicated env tests. Created a "standard'
and applied it to an other test also using `it.each`.
2025-02-26 17:04:52 +01:00
Paul Rastoin ec87218b9c [Refactor] generateDepthOneRecordGqlFieldsFromRecord (#10499)
## Introduction
This refactor results from this
https://github.com/twentyhq/twenty/pull/10493 review
Introduced a new abstraction to the extinsting
`generateDepthOneRecordGqlFields` that was accepting an optional record
in arg in order to map generated `recordGqlFields` to the keys in the
record

1/ Created a dedicated util method
`generateDepthOneRecordGqlFieldsFromRecord` to do so
2/ Updated each previous `generateDepthOneRecordGqlFields` passing a
record to call new `generateDepthOneRecordGqlFieldsFromRecord`
2025-02-26 11:11:45 +00:00
Paul Rastoin 611e89e5c9 [BUGFIX] Create optimistic cache generate recordGqlFields from prefilled record (#10493)
# Introduction
The record does not appear in the table because the optimistic record
input cached does not fulfill the mutation response fields, which result
in it being considered as invalid response
close #10199 
close https://github.com/twentyhq/twenty/issues/10153
2025-02-26 11:11:01 +01:00
89e11b4626 [BUGFIX] Account owner should not be clickable & [Refactor] Chip.tsx links (#10359)
# Introduction

closes #10196 
Initially fixing the `Account Owner` record field value should not be
clickable and redirects on current page bug.
This has been fixed computing whereas the current filed is a workspace
member dynamically rendering a stale Chip components instead of an
interactive one

## Refactor
Refactored the `AvatarChip` `to` props logic to be scoped to lower level
scope `Chip`.
Now we have `LinkChip` `Chip`, `LinkAvatarChip` and `AvatarChip` all
exported from twenty-ui.

The caller has to determine which one to call from the design system

## New rule regarding chip links
As discussed with @charlesBochet and @FelixMalfait 
A chip link will now ***always*** have `to` defined. ( and optionally an
`onClick` ).
`ChipLinks` cannot be used as buttons anymore

## Factorization
Deleted the `RecordIndexRecordChip.tsx` file ( aka
`RecordIdentifierChip` component ) that was duplicating some logic,
refactored the `RecordChip` in order to handle what was covered by
`RecordIdentifierChip`

## Conclusion
As always any suggestions are more than welcomed ! Took few opinionated
decision/refactor regarding nested long ternaries rendering `ReactNode`
elements

## Misc


https://github.com/user-attachments/assets/8ef11fb2-7ba6-4e96-bd59-b0be5a425156

---------

Co-authored-by: Mohammed Razak <mohammedrazak2001@gmail.com>
Co-authored-by: Félix Malfait <felix.malfait@gmail.com>
2025-02-25 14:36:17 +00:00
Paul Rastoin 68cb94f365 [BUGFIX] Call and await destroyDataSourceForWorkspace to avoid killing poor pod (#10364)
# Motivations
Upgrade migration is not possible atm in production because running over
all workspaces cause cpu raise leading to pod termination
2025-02-20 16:45:21 +01:00
Paul Rastoin 422e4e33c0 [BUGFIX][PROD] RICH_TEXT_V2 command handle {} body col value (#10324)
# Introduction
Encountered in issue in production where we have a lot of records that
has RICH_TEXT_FIELD set to `{}`

```sh
[Nest] 20106  - 02/19/2025, 12:43:08 PM     LOG [MigrateRichTextFieldCommand] Generating markdown for 1 records
[Nest] 20106  - 02/19/2025, 12:43:09 PM     LOG [MigrateRichTextFieldCommand] Error in workspace 3b8e6458-5fc1-4e63-8563-008ccddaa6db: TypeError: o is not iterable
```

## Fix
While reading `fieldValue` definition also strictly check if it's `{}` +
checking after JSON parse if it's an iterable to pass to the
`serverBlockNoteEditor` in order to be 100 bullet proof for prod
migration command

## Refactor Dry run
Implemented dry run

## Refactor to Idempotency
Made the script idempotent in order to avoid issues with re-running
commands

## Error repro
- In local checkout on v0.41.5 run `yarn && npx nx reset && npx nx
start`
- Create record manually in db that has a RICH_TEXT body to `{}`
- Checkout to main, `yarn && npx nx reset && npx nx build twenty-server
&& yarn command:prod upgrade-0.42:migrate-rich-text-field -d`
2025-02-20 09:16:58 +00:00
Paul Rastoin cd8e240311 [NITPICK] call closeDropdown before openModal import spreadsheet (#10254)
# Introduction
Just a nitpick resulting from
https://github.com/twentyhq/twenty/pull/10205
Avoid any possible race condition between competing event handlers by
closing dropdown before opening modal
2025-02-17 19:14:46 +01:00
Paul Rastoin f795d90702 [WEBSITE] Upgrade guide 0.41.0 -> 0.42.0 (#10231)
# Introduction
Upgrade guide from 0.41.0 to 0.42.0

## Note
@guillim I've been searching for references to the new `env vars` added
in the Microsoft scope to the version to the `setup.mdx` page, unless
I'm mistaken `IS_MICROSOFT_SYNC_ENABLED` seems to be missing ? I'm not
sure if it's required or not and on where to add it within the page,
could you please give it a look 🙏 ?
2025-02-15 09:11:57 +01:00
Paul Rastoin db1b11742b [0.42][FIX] Seeder activate rich text v2 feature flag (#10229)
as per title
2025-02-14 17:57:32 +00:00
Paul Rastoin 968ad3bd31 [NITPICK] From args list to record args RecordPositionFactory (#10215)
# Introduction
Just some nitpicking while debugging a bug that wasn't one
2025-02-14 15:43:09 +01:00
Paul Rastoin 5963c0f384 [REFACTOR][BUG] Dynamically compute field to write in cache CREATE (#10130)
# Introduction
While importing records encountering missing expected fields when
writting a fragment from apollo cache

## Updates

### 1/ `createdBy` Default value
When inserting in cache in create single or many we will now make
optimistic behavior on the createdBy value

### 2/ `createRecordInCache` dynamically create `recordGrqlFields`
When creating an entry in cache, we will now dynamically generate fields
to be written in the fragment instead of expecting all of them. As by
nature record could be partial

### 3/ Strictly typed `RecordGqlFields`

# Conclusion
closes #9927
2025-02-13 17:43:54 +01:00
Paul Rastoin 47f262c970 [REFACTOR][BUG] Dynamically compute field to write in cache UPDATE & DELETE (#10079)
# Introduction
At the moment when updating any record cache occurence, we will build a
fragment that will expect all of the object metadata item fields to be
provided.
Which result in the following traces: ( in the video companies aren't
fetch with companyId and other missing fields )


https://github.com/user-attachments/assets/56eab7c1-8f01-45ff-8f5d-78737b788b92

By definition as we're using graphql we might not request every record's
fields each time we wanna consume them.
In this way we will now dynamically compute or expect depending on the
CRUD operation specific fields to be written in the cache, and not all
of them

Tested all optimistic and failure management use cases

## Covering cache
Added coverage only for the `deleteOne` and `deleteMany` hooks, it cover
only the record record cache and not its relations hydratation ( for the
moment )

## Why not closing #9927 
Unless I'm mistaken everything done here have fixed the same logs/traces
issue for updates and deletion but not creation.
Which means we still need to investigate the mass upload from import and
prefillRecord behavior

In a nutshell: went over each `updateRecordFromCache` calls, still need
to do all `createRecordInCache` calls

related to #9927 

## Conlusion
Sorry for the big PR should have ejected into a specific one for the
`MinimalRecord` refactor
Will also continue covering others hooks later in my week as for the
`deleteOne`
As always any suggestions are welcomed !
2025-02-11 16:21:44 +01:00
Paul Rastoin aee43b10ad [BUG] Avoid backfilling record position on update one (#10069)
# Introduction
Disabled backfilling position process on single record update

It might be a too global solution ? feels a bit too easy tbh
Please let me know, could not think of other side-effects

close [#9925](https://github.com/twentyhq/twenty/issues/9925)
2025-02-07 14:19:58 +00:00
Paul Rastoin f31ee88fe8 [NITPICK] Avoid duplicated action position index in DefaultActionsConfigV2 (#10031)
# Introduction Nitpick
Action's record has two entries with `position: 0`
Unless I'm mistaken I could not find any place where the `position`
property is consumed
So this is just for sake of the +1 suite
2025-02-05 16:20:57 +01:00
Paul Rastoin dadee81175 [FEAT] Destroy many Workflow actions (#10028)
# Introduction
Added destroy many records to the `WorkflowActionConfig`

## Repro:
- Filter by deleted workflow
- Select mulitple rows
- Click on destroy

# Pinned action standardization
After discussion with @Bonapara and @bosiraphael we landed on a
standardization of the pinned actions between `DefaultActionsConfigV2`
and `WorkflowActionConfig` which means that now are pinned the `DELETE`
and `DESTROY` for both `single/multiple` modes for both features

Related to https://github.com/twentyhq/twenty/pull/9991
2025-02-05 16:16:13 +01:00
Paul Rastoin 710366514c BaseObjectRecord and__typename naming updates (#10024)
Folllowing:
https://github.com/twentyhq/twenty/pull/10014/commits/9049c47fd24c55dc7b03227a99587aadf651854d
From PR https://github.com/twentyhq/twenty/pull/10014
2025-02-05 14:51:30 +01:00
Paul Rastoin c3c800b097 Post #10014 merged nitpicks followup (#10021)
## Only nitpicks
Method signature mutation from several to one record arg
Renamed a variable in map to fit related business logic

#10014 Followup
2025-02-05 14:28:17 +01:00
Paul Rastoin 3e05c3743e [BUG][PROD] Fix ViewGroup creation optimistic cache (#10014)
# Introduction
When we create a new `view` from record table that has relation such as
opportunities.
Encountered invariant conditions:

## Unknown fiel `__typename`
`Should never occur, encountered unknown fields __typename in
objectMetadaItem viewGroup`,

### Fixed by ignoring unknown internal fields


## Provided both relation `view` and `viewId`
`Should never provide relation mutation through anything else than the
fieldId e.g companyId and not company, encountered: view`

### Fixed by sending only `viewId` to `createManyRecords` in
`usePersistViewGroupRecords.ts`
2025-02-05 12:22:45 +01:00
Paul Rastoin 28a3f75946 [FEAT] RecordAction destroy many record (#9991)
# Introduction
Added the `RecordAction` destroy multiple record

## Repro
Select multiples `deletedRecords`, you should be able to see the
`Destroy` pinned CTA ( iso short label with the destroy one ), open
control panel and fin new CTA `Permanently delete records`


https://github.com/user-attachments/assets/31ee8738-9d61-4dec-9a1f-41bb6785e018

## TODO
- [ ] Gain granularity within tests to assert the action should be
registered only when filtering by deleted

## Conclusion
Closes https://github.com/twentyhq/core-team-issues/issues/110
2025-02-05 11:33:01 +01:00
Paul Rastoin aa003f25d9 [CHORE] Twenty emails deps to twenty-shared lockfile update (#10013)
Related to https://github.com/twentyhq/twenty/pull/10010
Forgot to add lockfile
2025-02-05 10:28:11 +01:00
Paul Rastoin 0a8d75bc07 [FIX] Nx project's scope build dependency management (#10010)
# Introduction
Defined `dependsOn` for each nx project's configuration that has a
dependency to another local package ( ui, shared ).
As follows:
```json
"dependsOn": ["^build"]
``` 
Where the `^` symbol means "all dependencies of this project"

Now on a fresh repo, no built or install deps after install dependencies
you can directly hit `npx nx build PROJECT_NAME`
closes https://github.com/twentyhq/core-team-issues/issues/371

Related what was failing
[run](https://github.com/twentyhq/twenty-infra/actions/runs/13141544809/job/36669643182)
Cancelled before deploy, attested build was correct within the publish
and digest
2025-02-04 18:46:16 +01:00
Paul Rastoin 7fd89678b7 [CHORE] Avoid isDefined duplicated reference, move it to twenty-shared (#9967)
# Introduction
Avoid having multiple `isDefined` definition across our pacakges
Also avoid importing `isDefined` from `twenty-ui` which exposes a huge
barrel for a such little util function

## In a nutshell
Removed own `isDefined.ts` definition from `twenty-ui` `twenty-front`
and `twenty-server` to move it to `twenty-shared`.
Updated imports for each packages, and added explicit dependencies to
`twenty-shared` if not already in place

Related PR https://github.com/twentyhq/twenty/pull/9941
2025-02-01 12:10:10 +01:00
Paul Rastoin 385bf591cf [FIX] Storybook race condition (#9936)
Flaky test can be found
[here](https://github.com/twentyhq/twenty/actions/runs/13052735697/job/36416799488)
Action should be outside the waitFor callback

## Context
It seems to occurs since the Charles tooltip perf refactor
https://github.com/twentyhq/twenty/commit/ae4bf8d9298ca77cc45f6f4b0c25b66b51ad834f
Maybe the tests render being faster it changed order of execution ? idk
still interesting
2025-01-30 16:57:53 +01:00
Paul RastoinandCharles Bochet 9635fe9222 [BUG] Handle optimistic cache deletion operation (#9914)
# Introduction
This PR is highly related to previous optimistic cache refactor:
https://github.com/twentyhq/twenty/pull/9881
Here we've added some logic within the
`triggerUpdateRelationsOptimisticEffect` which will now run if given
recordInput `deletedAt` field is defined.

If deletion, we will iterate over all the fields searching for
`RELATION` for which deletion might implies necessity to detach the
relation

## Known troubleshooting ( also on main )

![image](https://github.com/user-attachments/assets/10ad59cd-e87b-4f26-89df-0a028fcfa518)
We might have to refactor the `prefillRecord` to spread and
overrides`inputValue` over defaultOne as inputValue could be a partial
one for more info please a look to

# Conclusion
Any suggestions are welcomed !
fixes https://github.com/twentyhq/twenty/issues/9580

---------

Co-authored-by: Charles Bochet <charles@twenty.com>
2025-01-30 11:12:37 +01:00
Paul Rastoin 85ae7084ab [FiX] Optimistic cache useUpdateOneRecord (#9913)
# Introduction
~~this could be cool for the updateRecordFromCache to return the updated
record, so we could consume its return value more explicitly such as
`updatedRecordValue`~~

In fact this is nitpick as it would really be the same than returning
the received params

## Fixes duplicated optimistic cache upsert
<img
src="https://github.com/user-attachments/assets/289f4801-5b67-4e4e-a87b-a512deb5180c"
width="300px" height="300px">

We were before comparing the previously `cachedRecord` which is now, not
the latest as the optimistic is (should) be working successfully since
https://github.com/twentyhq/twenty/pull/9881

## Reproduction
1-  Open any Company
2- Attach an Opportunity it should appears only once
3bis- If you revert the last branch commit it will appear duplicated

Related to #9580
2025-01-29 17:11:36 +01:00
Paul RastoinandCharles Bochet 29745c6756 [BUG] Fix record relation optimistic mutation (#9881)
# Introduction
It seems like optimistic caching isn't working as expected for any
record relation mutation, CREATE UPDATE DELETE.
It should not have an impact on the destroy

We included a new `computeOptimisticRecordInput` that will calculate if
a relation is added or detach.

Updated the `triggerCreateRecordsOptimisticEffect` signature we should
have a look to each of its call to determine if it should be checking
cache or not

Related to #9580

---------

Co-authored-by: Charles Bochet <charles@twenty.com>
2025-01-29 16:00:59 +01:00
Paul Rastoin c89cc38729 [MISC] Twenty-shared build declaration and declarationMap tsconfig (#9838)
# Motivations
By adding the declaration and declaration mapping this allows a better
`go to source` in our IDE
Should also be done for
https://github.com/twentyhq/core-team-issues/issues/281

## FROM


https://github.com/user-attachments/assets/5cc307d0-b2dc-46bf-b61f-0731015c4a30


## TO


https://github.com/user-attachments/assets/5ed1d7af-2716-435d-a1b8-6738b5a77956

## Notes
Might be interesting to add global commands for:
- watch mode for `twenty-ui` and `twenty-shared`
- clean, output folders such as `dist` and `node_modules` and so on
2025-01-24 17:56:42 +01:00
Paul Rastoin 95c772664e Refactor triggerUpdateRelationsOptimisticEffect to compute relationship from Metadatas (#9815)
# Introduction
At the moment the relationships are inferred from the record data
structure instead of its metadatas
We should refactor the code that computes or not the necessity to detach
a relation on a mutation

We've refactored the `isObjectRecordConnection` method to be consuming a
`relationDefintion` instead of "typeChecking" at the runtime the data
structure using zod validation schema

Related to #9580
2025-01-24 17:24:23 +01:00
Paul Rastoin 984dc4dec0 [MISC] Twenty-emails as package (#9770)
Charles has done everything within
https://github.com/twentyhq/twenty/pull/9754
Last thing to be done is removing the custom module `path` configured in
the `tsconfig.json`

close https://github.com/twentyhq/core-team-issues/issues/282
2025-01-22 16:33:48 +01:00
Paul RastoinandCharles Bochet 8ab01ebef4 [BUG] Record settings not saved (#9762)
# Introduction
By initially fixing this Fixes #9381, discovered other behavior that
have been fix.
Overall we encountered a bug that corrupts a workspace and make the
browser + api crash
This issue https://github.com/twentyhq/core-team-issues/issues/25
suggests a refactor that has final save button instead of auto-save

## `labelIdentifierFieldMetadataId` form default value
The default value resulted in being undefined, resulting in react hook
form `labelIdentifierFieldMetadataId` is required field error.

### Fix
Setting default value fallback to `null`  as field is `nullable`

## `SettingsDataModelObjectSettingsFormCard` never triggers form
Unless I'm mistaken in production touching any fields within
`SettingsDataModelObjectSettingsFormCard` would never trigger form
submission until you also modify `SettingsDataModelObjectAboutForm`
fields

### Fix
Provide and apply `onblur` that triggers the form on both
`SettingsDataModelObjectSettingsFormCard` inputs

## Wrong default `labelIdentifierFieldMetadataItem` on first page render
When landing on the page for the first time, if a custom
`labelIdentifierFieldMetadataItem` has been set it won't be computed
within the `PreviewCard`.
Occurs when `labelIdentifierFieldMetadataId` form default value is
undefined, due to `any` injection.

### Fix
In the `getLabelIdentifierFieldMetadataItem` check the
`labelIdentifierFieldMetadataIdFormValue` definition, if undefined
fallback to current `objectMetadata` identifier

---------

Co-authored-by: Charles Bochet <charles@twenty.com>
2025-01-22 16:32:57 +01:00
Paul Rastoin e0ea5b35d9 [SERVER] Typecheck the twenty-server (#9736)
# Introduction
It seems like that within the `twenty-server` `nx-project` there's no
entry for the `typecheck` command. It also seems to silently fail along
the `start` command even if it depends on it.
Looking at this
[run](https://github.com/twentyhq/twenty/actions/runs/12865329787/job/35865644811?pr=9736)
we can see that even when containing following diff it still does not
fail
```ts
// twenty-server/main.ts
// @ts-expected-error: Manually triggering TSC error should be deleted afterwards
const bootstrap = async () => {
```

By adding the `typecheck` entry in the project the same run will now
[fails](https://github.com/twentyhq/twenty/actions/runs/12865516249/job/35866224325)

![image](https://github.com/user-attachments/assets/e7d29307-05d0-4614-8e0f-b395129c3602)


# Notes
- There're no existing TSC errors even if wasn't guarded by the CI rn 😎 
- The ci will take dozen of more seconds to run, should keep an eye on
// with nx implem checking if it works as expected
2025-01-20 16:46:50 +01:00
Paul Rastoin 55feeaeef1 [CI][FIX] Storybook coverage configuration injection per scope (#9711)
# Introduction
Running `storybook` with `configuration=pages` outputs a coverage that
is not replicated afterwards by the `storybook:coverage` command.
```sh
npx nx storybook:serve-and-test:static twenty-front --configuration=pages --shard=1/1 --checkCoverage=true
# ...
[TEST] Test Suites: 40 passed, 40 total
[TEST] Tests:       52 passed, 52 total
[TEST] Snapshots:   0 total
[TEST] Time:        84.786 s
[TEST] Ran all test suites.
[TEST] Coverage file (13067196 bytes) written to .nyc_output/coverage.json
[TEST] > nx storybook:coverage twenty-front --coverageDir=coverage/storybook --checkCoverage=true
[TEST] 
[TEST] 
[TEST] > nx run twenty-front:"storybook:coverage" --coverageDir=coverage/storybook --checkCoverage=true
[TEST] 
[TEST] > npx nyc report --reporter=lcov --reporter=text-summary -t coverage/storybook --report-dir coverage/storybook --check-coverage=true --cwd=packages/twenty-front
[TEST] 
[TEST] 
[TEST] =============================== Coverage summary ===============================
[TEST] Statements   : 70.45% ( 775/1100 )
[TEST] Branches     : 45.39% ( 197/434 )
[TEST] Functions    : 63.52% ( 209/329 )
[TEST] Lines        : 71.28% ( 767/1076 )
[TEST] ================================================================================
[TEST] 
```

```sh
> npx nyc report --reporter=lcov --reporter=text-summary -t coverage/storybook --report-dir coverage/storybook --check-coverage=true --cwd=packages/twenty-front


=============================== Coverage summary ===============================
Statements   : 37.4% ( 9326/24931 )
Branches     : 22.99% ( 2314/10063 )
Functions    : 28.27% ( 2189/7741 )
Lines        : 37.81% ( 9261/24488 )
================================================================================
ERROR: Coverage for lines (37.81%) does not meet global threshold (39%)
ERROR: Coverage for branches (22.99%) does not meet global threshold (23%)
ERROR: Coverage for statements (37.4%) does not meet global threshold (39%)
Warning: command "npx nyc report --reporter=lcov --reporter=text-summary -t coverage/storybook --report-dir coverage/storybook --check-coverage=true --cwd=packages/twenty-front" exited with non-zero status code
```

## Fix
Persist configuration scope arg to the `check-coverage` command

## Question
Should we add a step in the `ci-front` what would merge all
`performance,modules,pages` coverage and calculate the `global` coverage
? => I think that this has no plus value as we still compute each of
them individualy
2025-01-17 12:42:32 +01:00
Paul Rastoin 0357797612 [CI] Add performance to merge coverage matrix strategy job (#9708)
# Introduction
It seems like I've just oversight adding it back while debugging

# Centralization
Please note that we only have access to the following context within a
job matrix properties:
```
Available expression contexts: github, inputs, vars, needs
```
We could centralize the `storybook_scope` as a job `outputs` in order to
avoid this to re-occurs.
2025-01-17 11:17:39 +01:00
Paul Rastoin 22ee77145a [CI] ci-server reorder twenty-shared build after cache restoration (#9672)
# Introduction
Unless I'm mistaken we can win few seconds `~12s` by reordering the
`twenty-shared` build step after the cache restoration in order for `nx`
to hit it if possible.

[Related
run](https://github.com/twentyhq/twenty/actions/runs/12809421832/job/35714544486)
2025-01-17 08:06:30 +01:00
Paul Rastoin 560f715c37 [CI][NITPICK]: Rename prerequisites jobs to changed-files-check (#9670)
Related to https://github.com/twentyhq/twenty/pull/9643

Renaming `prerequisites` jobs to a more accurate `changed-files-check`
2025-01-16 14:09:59 +01:00
Paul Rastoin f8ddc02b8e [CI] Refactor changed files integration (#9643)
# BEFORE
[run](https://github.com/twentyhq/twenty/actions/runs/12806801953)

![image](https://github.com/user-attachments/assets/f0a8ffe3-3fc0-42ca-b2ee-8a980606b5dd)
# AFTER
[run](https://github.com/twentyhq/twenty/actions/runs/12807034402)

![image](https://github.com/user-attachments/assets/5117c680-6804-416b-a8c8-bf00614ca453)

## Motivations:
- less workflow to whitelist as blocking for PRs
- less if condition per step

cons:
- quite verbose
- need to manually sync the `ci-NAME-status-check` needs list to any
other existing and should be dep jobs

## Version migration
Migrated to the latest `changed-files@45` version, getting rid of the
`set-output` usage warnings

## Tests runs:

With mutation:
- [Success
flow](https://github.com/twentyhq/twenty/actions/runs/12791958651/job/35661546343)
- [server-setup failure
flow](https://github.com/twentyhq/twenty/actions/runs/12792225779)
- [Other job failure
flow](https://github.com/twentyhq/twenty/actions/runs/12792313463), one
of the `inner` job failed
- [Manual cancel
flow](https://github.com/twentyhq/twenty/actions/runs/12792313463)
`ci-server-status-check` also has the `cancelled` status
- [Matrix
failure](https://github.com/twentyhq/twenty/actions/runs/12806883553)

Without mutation:
- [Nothing to do
flow](https://github.com/twentyhq/twenty/actions/runs/12792098384),
skipped `inner` job but `ci-server-status-check` still succeeded

## Notes
### Linter
We should setup a `yml` prettier and linter for the `.github/worfklows`
folder
### Centralized `ci-NAME-status-check` logic
Unfortunately I couldn't achieve to either make a `composite` action or
a `reusable-workflow`, as I could not access the correct layer to run
the `always` but also acessing the `needs` context
2025-01-16 13:37:28 +01:00
Paul Rastoin 6a738181b4 [CI][NITPICK] .yml to yaml github workflows (#9664)
# Introduction
Applying the most present `yaml` file extension to all
`.github/workflows/` files

## Notes
Regarding the `restore-cache` and `save-cache` composite actions github
agnostically searches for any `DockerFile` `action.yml` and
`action.yaml` within the target folder when invoking composite actions
as follows:
```yml
      - name: Restore storybook build cache
        if: steps.changed-files.outputs.any_changed == 'true'
        uses: ./.github/workflows/actions/restore-cache
        with: 
          key: ${{ env.STORYBOOK_BUILD_CACHE_KEY }}
```
2025-01-16 11:21:00 +01:00
Paul Rastoin d63aec44bb [CI] Yarn install composite action remove --immutable-cache (#9641) 2025-01-15 16:02:29 +01:00
Paul Rastoin 7cb259d692 [CI] Refactor yarn-install composite action (#9613)
# Introduction
Unless I'm mistaken in the existing `yarn-install` composite action
`dependencies` are cached twice.

## Cached through `setup-node`
As we're providing `yarn` value to cache inputs.
Setup node will cache the global `.yarn`
```yml
    - name: Setup Node.js and get yarn cache
      uses: actions/setup-node@v3
      with:
        node-version: 18
        cache: yarn
```

## Programmatic `node_modules` caching
We even manually still cache `node_modules` folder using:
```yml
    - name: Cache node modules
      id: node-modules-cache
      uses: actions/cache@v3
      with:
        path: |
          node_modules
          packages/*/node_modules
        key: root-node_modules-${{ hashFiles('yarn.lock') }}
        restore-keys: root-node_modules-
```
By the way it seems like that the `yarn.lock` file is useless as the
restore-key pattern omits it
This could, unless I'm mistaken, leads to invalid cache restore

## Runtime
The current infra results in the following install deps logs:
```sh
Prepare all required actions
Getting action download info
Download action repository 'actions/setup-node@v3' (SHA:1a444[2](https://github.com/twentyhq/twenty/actions/runs/12770942233/job/35596946506#step:6:2)cacd436585916779262731d5b162bc6ec7)
Download action repository 'actions/cache@v[3](https://github.com/twentyhq/twenty/actions/runs/12770942233/job/35596946506#step:6:3)' (SHA:f4b3439a656ba812b8cb417d2d49f9c810103092)
Run ./.github/workflows/actions/yarn-install
Run actions/setup-node@v3
Found in cache @ /opt/hostedtoolcache/node/18.20.5/x6[4](https://github.com/twentyhq/twenty/actions/runs/12770942233/job/35596946506#step:6:4)
Environment details
/usr/local/bin/yarn --version
4.4.0
/usr/local/bin/yarn config get cacheFolder
/home/runner/.yarn/berry/cache
/usr/local/bin/yarn config get enableGlobalCache
true
Cache Size: ~421 MB (441369819 B)
/usr/bin/tar -xf /home/runner/work/_temp/883eae34-9b21-4684-96[5](https://github.com/twentyhq/twenty/actions/runs/12770942233/job/35596946506#step:6:5)8-cdb937f62965/cache.tzst -P -C /home/runner/work/twenty/twenty --use-compress-program unzstd
Cache restored successfully
Cache restored from key: node-cache-Linux-yarn-a8c18b6600f1bc685e60192f39a0a0c5c51b918829090129d3787302789547fc
Run actions/cache@v3
Cache Size: ~506 MB (530305961 B)
/usr/bin/tar -xf /home/runner/work/_temp/becd3767-ac80-4ca9-8d[21](https://github.com/twentyhq/twenty/actions/runs/12770942233/job/35596946506#step:6:23)-93fa61e3070c/cache.tzst -P -C /home/runner/work/twenty/twenty --use-compress-program unzstd
Cache restored successfully
Cache restored from key: root-node_modules-a8c18b6600f1bc685e60192f39a0a0c5c51b918829090129d378730[27](https://github.com/twentyhq/twenty/actions/runs/12770942233/job/35596946506#step:6:30)89547fc
```
Where we can see in details that a cache is hit twice.
```
Cache Size: ~421 MB (441369819 B)
Cache Size: ~506 MB (530305961 B)
```

## Suggestion
We should stick to only one deps caching mechanism.
Also caching the node_modules folder is not
[recommended](https://github.com/actions/cache/blob/611465405cc89ab98caca2c42504d9289fb5f22e/examples.md#node---npm).
That's why I would factorize our caching to the `setup-node` scope only.

## The cache-primary-key crafting
Within the cache-primary-key we will inject the following metrics:
- Node version, as we're caching `node_modules` directly in this way to
avoid cache desync in case we upgrade node.
- Hash of the lockfile, in this way if the lockfile comes to mutate we
will re-install the deps from scratch, in this way no need to
programmatically listen for `changed-files` on the `package.json` and
the `lockfile`
- Strict installation with `check-cache` and `enableHardenedMode` to
true and obviously the `--immutable` flag ( note adding the
`--immutable-cache` by principle even if on paper is overkill as a cache
restoration and install should never occurs )
2025-01-15 15:46:53 +01:00
Paul Rastoin 9ba510eb3f [CI] Always save cache agnostically of the actions/restore cache-hit output (#9635)
## Motivations
This is not because we've hit the `actions/cache/restore` cache that NX
won't be re-building sub commands.
Which means the grain to save or not the cache should be extracted from
nx sub commands and not from the `actions/cache` outputs.

We should investigate on the way to retrieve this granularity level

From the moment we will be saving everything, which could result
sometimes in duplicating cache instances.
We should keep in mind that our cache occurrences have a 1 day retention
date, is low cost and pretty fast to perform
2025-01-15 11:59:17 +01:00
Paul Rastoin d133d7f10e [CI] Migrate to ubuntu 24.04 runners (#9607)
## Introduction
Our GitHub Runners provider just
[released](https://depot.dev/changelog/2024-07-12-ubuntu-24-04-github-actions-runner-beta)
ubuntu-24.04 runners lately

## Cache primary key factorization
Refactored the calculation of the cache-primary-key prefix to avoid
duplication

## Runs
- twenty-front lint
https://github.com/twentyhq/twenty/actions/runs/12770312293/job/35595272862
- twenty-server
https://github.com/twentyhq/twenty/actions/runs/12770218595
2025-01-14 16:18:45 +01:00
Paul Rastoin b815de3e26 [FIX][CI] cache-hit incorrectly computed (#9603)
## Introduction
The `cache-hit` value should be a string boolean, but it seems like not
to always be the case ?

## Related runs
- twenty-server first attempt saves cache second retrieves it
https://github.com/twentyhq/twenty/actions/runs/12768260350
- twenty-front
https://github.com/twentyhq/twenty/actions/runs/12767064201
2025-01-14 14:18:33 +01:00
Paul Rastoin 5d09bd4b93 [CI][NITPICK] Remove input types invalid syntax (#9596)
## Introduction
Unless I'm mistaken neither the `inputs.type` or `inputs.type` syntax
exists for `composite actions`.
By default it seems like any `Composite action` inputs are considered as
`string`.
```yml
name: Nx Affected CI
inputs:
  parallel:
    required: false
    types: [number]
    default: 3
  tag:
    required: false
    types: [string]
  tasks:
    required: true
    types: [string]
```

## Suggestion
To avoid misunderstanding of our composite actions inputs signature I
would recommend removing any `type` props but also default any inputs as
`strings`

## Misc
- Please find related GitHub community discussion
https://github.com/orgs/community/discussions/65588
2025-01-14 12:23:37 +01:00
Paul Rastoin 6fc691beb0 [CI] Refactor composite action task-cache (#9583)
## Introduction
In this previous PR https://github.com/twentyhq/twenty/pull/9448 we've
refactored the storybook build caching flow to be using the new
[actions/cache](https://github.com/actions/cache) `restore` and `save`
functions, which significantly improve caching operations duration.

In this way, in this PR, we've standardize both of the `restore` and
`save` by refactoring the `task-cache` composite action. By creating two
new composite actions `save-cache` and `restore-cache` that centralize
the paths to cache and the way to compute the primary key.

## Misc
- **If no cache** is hit, then a job duration will long for its task
duration and nothing else, previously the cache upload would sometimes
take up to 3 mins.
- **if cache** is hit, then mainly the only time consuming step is the
dependencies installation ( which is theory is also cached, in fact
twice. We will be having a look on this issue in an upcoming PR )
2025-01-14 12:06:23 +01:00
Paul Rastoin 873f20bc0e [CI][FRONT] Storybook tests sharding (#9448)
# Introduction
The idea here is to improve perf nor fluidity of both storybook build
and tests execution duration.

## Levers:
- Storybook tests
[shards](https://storybook.js.org/docs/writing-tests/test-runner)
- Refactored storybook's build caching using `actions/cache/restore` and
`actions/cache/save` to avoid useless computation with we wanna just
write or retrieve it
- Running storybook build with --test
[flag](https://storybook.js.org/docs/api/main-config/main-config-build)
( please note that we only disable docs addons in order to keep coverage
up and running )

closes https://github.com/twentyhq/core-team-issues/issues/49
2025-01-10 18:40:03 +01:00
Paul RastoinandPaul Rastoin aa0d8546a8 [REFACTOR][FRONT]: Remove objectMetadata and fieldMetadata sluggification (#9441)
# Introduction
For motivations and context please have a look to
https://github.com/twentyhq/twenty/pull/9394 whom this PR results from.
In this pull-request we remove any `metadataField` and `objectMetadata`
sluggification. We directly consume `objectMetadata.namePlural` and
`metadataField.name`, ***it seems like that historically the consumed
`metadataField.name`*** are we sure that we wanna change this behavior ?

## Notes
Unless I'm mistaken by reverting the `kebabcase` url formatting we might
be creating deadlinks that user could have save beforehand => Discussed
with Charles said it's controlled risk.

---------

Co-authored-by: Paul Rastoin <paulrastoin@Pauls-MacBook-Pro.local>
2025-01-08 11:31:53 +01:00
Paul RastoinandPaul Rastoin 7b70f7d93b [9382][FRONT]: Compute highlighted object view using raw name plural (#9394)
# Introduction
Please find related ticket here #9382
To fix the issue the solution seems to be to stop searching for last
viewed `objectMetadata` using their slugged version `namePlural`

## Upcoming cleanup
After discussing with @charlesBochet it seems like a bad practice to
slug the `objectMetadata`, in this way in a following PR we will suggest
a cleanup of the remaining method that does within the
`useFilteredObjectMetadataItems.ts`

## Conclusion
As always any suggestions are welcomed !
Please let me know

closes #9382

---------

Co-authored-by: Paul Rastoin <paulrastoin@Pauls-MacBook-Pro.local>
2025-01-07 15:20:54 +01:00