After discussing it with the team, we now want to query all fields in
the table and the board by default. Feeding the cache with exhaustive
data will make the side panel's life easier, as it needs all the record
fields to determine the actions to enable.
## What
- Deprecate overlayscrollbars as we decided to follow the native
behavior
- rework on performances (avoid calling recoil states too much at field
level which is quite expensive)
- Also implements:
https://github.com/twentyhq/core-team-issues/issues/569
---------
Co-authored-by: Lucas Bordeau <bordeau.lucas@gmail.com>
In this PR, I'm:
- fixing the root cause (we should not try to render a RecordChip if the
record is not defined in RelationFromMany Display)
- fixing related typing issues
- we won't be able to catch the issue from TS perspective as
ObjectRecord is a Record of string, any
# Introduction
In this PR we've migrated `twenty-shared` from a `vite` app
[libary-mode](https://vite.dev/guide/build#library-mode) to a
[preconstruct](https://preconstruct.tools/) "atomic" application ( in
the future would like to introduce preconstruct to handle of all our
atomic dependencies such as `twenty-emails` `twenty-ui` etc it will be
integrated at the monorepo's root directly, would be to invasive in the
first, starting incremental via `twenty-shared`)
For more information regarding the motivations please refer to nor:
- https://github.com/twentyhq/core-team-issues/issues/587
-
https://github.com/twentyhq/core-team-issues/issues/281#issuecomment-2630949682
close https://github.com/twentyhq/core-team-issues/issues/589
close https://github.com/twentyhq/core-team-issues/issues/590
## How to test
In order to ease the review this PR will ship all the codegen at the
very end, the actual meaning full diff is `+2,411 −114`
In order to migrate existing dependent packages to `twenty-shared` multi
barrel new arch you need to run in local:
```sh
yarn tsx packages/twenty-shared/scripts/migrateFromSingleToMultiBarrelImport.ts && \
npx nx run-many -t lint --fix -p twenty-front twenty-ui twenty-server twenty-emails twenty-shared twenty-zapier
```
Note that `migrateFromSingleToMultiBarrelImport` is idempotent, it's atm
included in the PR but should not be merged. ( such as codegen will be
added before merging this script will be removed )
## Misc
- related opened issue preconstruct
https://github.com/preconstruct/preconstruct/issues/617
## Closed related PR
- https://github.com/twentyhq/twenty/pull/11028
- https://github.com/twentyhq/twenty/pull/10993
- https://github.com/twentyhq/twenty/pull/10960
## Upcoming enhancement: ( in others dedicated PRs )
- 1/ refactor generate barrel to export atomic module instead of `*`
- 2/ generate barrel own package with several files and tests
- 3/ Migration twenty-ui the same way
- 4/ Use `preconstruct` at monorepo global level
## Conclusion
As always any suggestions are welcomed !
Fixes#11038
# Fix useFindManyRecords withSoftDeleterFilter
The error came from a faulty implementation of the `withSoftDeleted`
parameter inside `useFindManyRecords` and from the fact that
`withSoftDeleted: true` was added to access deleted records actions.
However, this parameter was always set in
`useFindManyRecordsSelectedInContextStore` instead of considering
whether the filter was active or not.
```
const withSoftDeleterFilter = {
or: [{ deletedAt: { is: 'NULL' } }, { deletedAt: { is: 'NOT_NULL' } }],
};
```
The final filter was incorrectly doing an `or` operation between the
base filter and `withSoftDeleterFilter` when it should have been an
`and`:
```
filter: {
...filter,
...(withSoftDeleted ? withSoftDeleterFilter : {}),
}
```
The correct implementation should be:
```
filter:
filter || withSoftDeleted
? {
and: [
...(filter ? [filter] : []),
...(withSoftDeleted ? [withSoftDeleterFilter] : []),
],
}
: undefined,
```
# Fix useFindManyRecordsSelectedInContextStore
- Check if the soft deleted filter is active before using the
`withSoftDeleterFilter` parameter
## 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
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
Workspace Member will get their own record page in the future.
This PR lays backend changes to prepare for this:
- Settings most fields on WorkspaceMember as system fields
- Renaming workspaceMember/workspaceMemberId to
forWorkspaceMember/forWorkspaceMemberId as it conflicts with the morph
relationship, if we want to be able to add a workspace member as
favorite
---------
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
In this huge (sorry!) PR:
- introducing objectMetadataItem in contextStore instead of
objectMetadataId which is more convenient
- splitting some big hooks into smaller parts to avoid re-renders
- removing Effects to avoid re-renders (especially onViewChange)
- making the view prefetch separate from favorites to avoid re-renders
- making the view prefetch load a state and add selectors on top of it
to avoir re-renders
As a result, the performance is WAY better (I suspect the favorite
implementation to trigger a lot of re-renders unfortunately).
However, we are still facing a random app freeze on view creation. I
could not investigate the root cause. As this seems to be already there
in the precedent release, we can move forward but this seems a urgent
follow up to me ==> EDIT: I've found the root cause after a few ours of
deep dive... an infinite loop in RecordTableNoRecordGroupBodyEffect...
prastoin edit: close https://github.com/twentyhq/twenty/issues/10253
---------
Co-authored-by: Lucas Bordeau <bordeau.lucas@gmail.com>
Co-authored-by: prastoin <paul@twenty.com>
# 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
## Context
All objects have '...duplicates' resolver but only companies and people
have duplicate criteria (hard coded constant).
Gql schema and resolver should be created only if duplicate criteria
exist.
## Solution
- Add a new @WorkspaceDuplicateCriteria decorator at object level,
defining duplicate criteria for given object.
- Add a new duplicate criteria field in ObjectMetadata table
- Update schema and resolver building logic
- Update front requests for duplicate check (only for object with
criteria defined)
closes https://github.com/twentyhq/twenty/issues/9828
# 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 !
Implements filtering, ordering and cursor filtering for the hook
useCombinedFindManyRecords, because it was not implemented, which was
misleading because variables could be passed to it.
The difficult part was to make sure that the cursor filtering was
working, both before and after a cursor, because it was only hard coded
for last cursor (equivalent to after).
The duplicate limit parameter in the type RecordGqlOperationVariables
was merged into one limit parameter, because it was making the developer
guess how both could be handled.
This single limit parameter can be used for either : general limit
without cursor, first records from after cursor, last records until
before cursor. Since those cases are exclusive it's better to have only
one limit parameter and have an internal logic handling those cases.
Tests were added on the relevant parts, especially
useCombinedFindManyRecordsQueryVariables which requires its own unit
test to handle this cursor + limit logic.
Record show page pagination was tested to make sure removing the
duplicate limit parameter had no impact.
# 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
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
Implementing the Outlook icon for CreatedBy, only for emails.
Not in this PR original scope : The similar feature for calendar created
records. Since it was straightforward, I added it to the scope of this
PR.
Fix https://github.com/twentyhq/core-team-issues/issues/252
# 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
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>
In this PR
- fixing Collapse on view groups views: aggregate bar should be included
in the collapse (@magrinj )
- respect the html table pattern: the aggregate bar is now a <tr>
element included in a <table> (before that, it was a <tr> not included
in anything)
- add a top-border on the aggregate bar
- introduce short labels for the on-cell value display (display "Empty"
instead of "Count empty" to lighten the interface)
- remove the feature flag !
The DX is not great when you need to do a lot of database
resets/command.
Should we disable Typescript validation to speed things up? With this
and caching database:reset takes 1min instead of 2 on my machine.
See also: https://github.com/typeorm/typeorm/issues/4136
And #9291 / #9293
---------
Co-authored-by: Lucas Bordeau <bordeau.lucas@gmail.com>
There are two follow ups to this PR:
- Bug: sometimes when opening Cmd+K from a deleted record, we are facing
a global error
- On Index page, actions in top right are displaying label and not short
name
- Implement multiple actions once refactoring on delete is complete
---------
Co-authored-by: bosiraphael <raphael.bosi@gmail.com>
