# Introduction
close https://github.com/twentyhq/core-team-issues/issues/487
Updated the seeders to infer the workspace's version from the
`APP_VERSION` env var
To test in local run: `npx nx database:reset twenty-server` with either
a defined or not defined `APP_VERSION` in your `.env`
( note that invalid semver values will throw an error and stop the
process ) ( valid version ex: `APP_VERSION=1.0.0`)
# Introduction
We want the APP_VERSION to be able to contains pre-release options, in a
nutshell to be semVer compatible.
But we want to have workspace, at least for the moment, that only store
`x.y.z` and not `vx.y.z` or `x.y.z-alpha` version in database
Explaining this refactor
Related https://github.com/twentyhq/twenty/pull/10907
# Introduction
Under the hood class-validator isSemver uses
https://github.com/validatorjs/validator.js/blob/master/src/lib/isSemVer.js
which does not cover all semVer use cases
## Even tho
Had a discussion with @charles was about to store in db ws version as
`vx.y.z`. We felt like we wanted it to be stored as `x.y.z`, in my
opinion `APP_VERSION` should reflect the tag used to be build the
instance and not be updated
But we could extract only `x.y.z` from it at runtime
Also handling the `v` extraction in CD is IMO not the most reliable
## Env var logging refactor
Now not stopping on first error log
```ts
Successfully compiled: 2128 files with swc (185.34ms)
Watching for file changes.
[Nest] 52686 - 03/14/2025, 6:28:33 PM ERROR PG_DATABASE_URL should not be null or undefined
PG_DATABASE_URL must be a URL address
[Nest] 52686 - 03/14/2025, 6:28:33 PM ERROR APP_VERSION must be a valid semantic version (e.g., 1.0.0)
/Users/paulrastoin/ws/twenty/packages/twenty-server/src/engine/core-modules/environment/environment-variables.ts:1019
throw new Error("Environment variables validation failed")
^
Error: Environment variables validation failed
at Object.validate (/Users/paulrastoin/ws/twenty/packages/twenty-server/src/engine/core-modules/environment/environment-variables.ts:1019:11)
at Function.forRoot (/Users/paulrastoin/ws/twenty/node_modules/@nestjs/config/dist/config.module.js:67:45)
at Object.<anonymous> (/Users/paulrastoin/ws/twenty/packages/twenty-server/src/engine/core-modules/environment/environment.module.ts:11:18)
at Module._compile (node:internal/modules/cjs/loader:1256:14)
at Object.Module._extensions..js (node:internal/modules/cjs/loader:1310:10)
at Module.load (node:internal/modules/cjs/loader:1119:32)
at Function.Module._load (node:internal/modules/cjs/loader:960:12)
at Module.require (node:internal/modules/cjs/loader:1143:19)
at require (node:internal/modules/cjs/helpers:121:18)
at Object.<anonymous> (/Users/paulrastoin/ws/twenty/packages/twenty-server/dist/src/database/typeorm/typeorm.module.js:14:28)
```
# Introduction
We want any new activated workspace to be filled with a version equal to
the current `APP_VERSION`
Please note that in a workspace lifecycle this operation will be run
only once, discussed with @charlesBochet in front of `3 fois plus de
piments`
Going straightforward in this PR in order to release asap
Started et will continue to implem new integrations regarding `SignUp`
and `ActivateWorkspace` happy and expections path in
https://github.com/twentyhq/twenty/tree/prastoin-new-workspace-has-version-integrations-tests
# Introduction
In https://github.com/twentyhq/twenty/pull/10751 we decided not to put
`APP_VERSION` references in `.env.example` as it's programmatically
defined by our CD and should not be override by any manual interaction.
Still, as a dev testing the upgrade command in local, if you do not set
the `APP_VERSION` in local you will encounter the following error:
```ts
'Cannot run upgrade command when APP_VERSION is not defined'
```
@guillim currently doing the release legitimately raised that it was not
very intuitive
## Levers
- Improve error message such as adding reference to checking env
variables
- App local upgrade command dev dedicated documentation ?
## Conclusion
Any suggestions are more than welcomed !
# Introduction
Refactored the upgrade command to be more intuitive to anyone wanting to
add a command to the next relase upgrade instance
Also updated the upgrade command for the next 0.44 release
# Introduction
This PR contains a big test file and few snapshots
Related to https://github.com/twentyhq/core-team-issues/issues/487
## New env var `APP_VERSION`
Now will be injected directly in a built docker image the twenty's built
version. Inferred from the build git tag name.
Which mean on main or other `not a tag version` built APP_VERSION will
be `null`
## New upgrade-commander-runner
Refactored the upgrade command to be more strict regarding:
- Version management
- Sync metadata command always run
- Added failing workspaces aggregator + logs on cleanup
From now on the `upgrade` command will compare the `WORKSPACE_VERSION`
to the `APP_VERSION` in order to bypass any workspace version != than
the upgrade version `fromVersion`
## Existing commands
Note that the version validation will be done only when passing by the
`upgrade` command.
Which means that running the following command
`upgrade:x.y-some-specific-command` won't result in workspace version
mutation
This is to enforce that all an upgrade commands + sync-metadata has been
run on a workspace
## Will do in other PR but related
### New workspace
New workspace will now be inserted with version equal to the APP_VERSION
they've been created by
### Old workspace
Will create a command that should be ran outside of any `upgrade-runner`
extending command, the command will have to be ran on every workspace
before making the next release upgrade
This command iterates over any active and suspended workspace that has
`version` to `NULL` in order to update it `APP_VERSION` -1 minor
### SENTRY_RELEASE
- Either deprecate SENTRY_RELEASE in favor of `APP_VERSION` => What
about main with null version ? or create a new env var that would be
`APP_COMMIT_SHA` instead of SENTRY third party ref
### Update CD to inject APP_VERSION from branch name
### Update docs and release logs
Adding documentation for `APP_VERSION`
## Related PRs:
https://github.com/twentyhq/twenty-infra/pull/181
# Introduction
This PR contains several SNAPSHOT files explaining big +
While refactoring the Object Model settings page in
https://github.com/twentyhq/twenty/pull/10653, encountered a critical
issue when submitting either one or both names with `""` empty string
hard corrupting a workspace.
This motivate this PR reviewing server side validation
I feel like we could share zod schema between front and back
## Refactored server validation
What to expect from Names:
- Plural and singular have to be different ( case insensitive and
trimmed check )
- Contains only a-z A-Z and 0-9
- Follows camelCase
- Is not empty => Is not too short ( 1 )
- Is not too long ( 63 )
- Is case insensitive( fooBar and fOoBar now rejected )
What to expect from Labels:
- Plural and singular have to be different ( case insensitive and
trimmed check )
- Is not empty => Is not too short ( 1 )
- Is not too long ( 63 )
- Is case insensitive ( fooBar and fOoBar now rejected )
close https://github.com/twentyhq/twenty/issues/10694
## Creation integrations tests
Created new integrations tests, following
[EachTesting](https://jestjs.io/docs/api#testeachtablename-fn-timeout)
pattern and uses snapshot to assert errors message. These tests cover
several failing use cases and started to implement ones for the happy
path but object metadata item deletion is currently broken unless I'm
mistaken @Weiko is on it
## Notes
- [ ] As we've added new validation rules towards names and labels we
should scan db in order to standardize existing values using either a
migration command or manual check
- [ ] Will review in an other PR the update path, adding integrations
tests and so on
# Introduction
This PR contains around ~+300 tests + snapshot additions
Please check both object model creation and edition
Closes https://github.com/twentyhq/core-team-issues/issues/355
Refactored into two agnostic forms the Object Model settings page for
instance `/settings/objects/notes#settings`.
## `SettingsDataModelObjectAboutForm`
Added a new abstraction `SettingsUpdateDataModelObjectAboutForm` to wrap
`SettingsDataModelObjectAboutForm` in an `update` context
Schema:
```ts
const requiredFormFields = objectMetadataItemSchema.pick({
description: true,
icon: true,
labelPlural: true,
labelSingular: true,
});
const optionalFormFields = objectMetadataItemSchema
.pick({
nameSingular: true,
namePlural: true,
isLabelSyncedWithName: true,
})
.partial();
export const settingsDataModelObjectAboutFormSchema =
requiredFormFields.merge(optionalFormFields);
```
## `SettingsDataModelObjectSettingsFormCard`
Update on change
Schema:
```ts
export const settingsDataModelObjectIdentifiersFormSchema =
objectMetadataItemSchema.pick({
labelIdentifierFieldMetadataId: true,
imageIdentifierFieldMetadataId: true,
});
```
## Error management and validation schema
Improved the frontend validation form in order to attest that:
- Names are in camelCase
- Names are differents
- Names are not empty string ***SHOULD BE DONE SERVER SIDE TOO*** ( will
in a next PR, atm it literally breaks any workspace )
- Labels are differents
- Labels aren't empty strings
Hide the error messages as we need to decide what kind of styling we
want for our errors with forms
( Example with error labels )
# Introduction
close#9965
When landing on twenty you should be able to see a white screen
flickering if you had setup dark mode.
This is because before the SPA has been loaded we're not displaying
anything, which in a white screen from the browser.
During this period we should display a background color following the
user's device theme.
## Reproduction
In order to reproduce this behavior define a fast 4G connection from
your network console.
## Cons
Device mode might not the one chosen afterwards when the user has been
authenticated
=> We should store appearance settings in the local storage in order to
optimistically render the default "loading" background ( wouldn't be
100% bullet proof for instance if the user is now unauth for some reason
)
Body background will be override by theme after app bootstrap
## Introduction
Added coverage on the `useDeleteOneRecord` hooks, especially its
optimistic behavior feature.
Introduced a new testing tool `InMemoryTestingCacheInstance` that has
builtin very basic expectors in order to avoid future duplication when
covering others record hooks `update, create, destroy` etc etc
## Notes
Added few comments in this PR regarding some builtin functions I've
created around companies and people mocked object model and that I think
could be cool to spread and centralize within a dedicated "class
template"
Also put in light that unless I'm mistaken some tests are running on
`RecordNode` and not `RecordObject`
Took few directions on my own that as I always I would suggestion nor
remarks on them !
Let me know
## Misc
- Should we refactor `useDeleteOneRecord` tests to follow `eachTesting`
pattern ? => I feel like this is inappropriate as this hooks is already
high level, the only plus value would be less tests code despite
readability IMO
# Introduction
Historically we've been programmatically running sync metadata just
after all upgrade command's migration.
Adding back this behavior as default to the new dynamic modules
Duplicated already existing synchronize metadata logic as a quick fix as
we're about to iterate over commands next sprint
## Introduction
This is PR is a suggestion ! And should be discussed
With yarn `^4`, during installation won't raise an error if current dev
env does not satisfies the `engines` policy.
We have usually 10+ contributors support request regarding higher node
version issue per week
I would have preferred a very declarative integration using npm
[engines](https://docs.npmjs.com/cli/v11/configuring-npm/package-json#engines)
but this does not seems to be natively supported by `yarn`
We should keep in mind that this might block any machines from our CICD
if they have diff node version installed ( such as running the project
on a different node version could result in bugs too )
## Implem
Created a yarn [constraints](https://yarnpkg.com/features/constraints)
run after each installation that checking if current node version
satisfies defined engines range ( might also be done for others engines
entries )
I assume we will always have the same engines policy for every packages,
at least that's not a consideration from now
## Further
We could refactor our package.json engines into only one using
`Yarn.set` etc
## Resource
- https://yarnpkg.com/configuration/yarnrc
- https://yarnpkg.com/features/constraints
## Note
- Not running constraints in `preInstall` hook as won't be effective on
fresh install
-
[engine-strict](https://docs.npmjs.com/cli/v8/using-npm/config#engine-strict)
is an npm-config
-
[devEngines](https://docs.npmjs.com/cli/v11/configuring-npm/package-json#devengines)
are npm feature too ( for instance pnpm current PR
https://github.com/pnpm/pnpm/issues/8153 )
## Conclusion
As always any suggestions are more than welcomed !
# 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`.
## 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`
# 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
# 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>
# 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`
# 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
# 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 🙏 ?
# 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
# 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 !
# 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)
# 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
# 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
# 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`
# 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
# 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
# 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
# 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 )
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>
# 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
# 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>
# 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
# 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>
# 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)
# 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
# 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
# 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.
# 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 }}
```
# 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 )
## 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
## 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
## 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 )
# 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>
# 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>
Paul Rastoin