In this PR, I'm fixing a bug introduced in recent performance work on
the cache.
Bug context: https://github.com/twentyhq/twenty/issues/12865
Related PR opened by a contributor:
https://github.com/twentyhq/twenty/pull/13003
## Root cause
We cache all objectMetadataItems at graphql level : see
`useCachedMetadata` hook:
- instead of going through the regular resolvers, we direlcty load data
from the cache. However this data must be localized regarding labels and
descriptions
In a precedent refactoring, we introduced the notion of locale in the
cache key. However, the user locale was not properly taken into account
as we did not have the information in this hook.
## Fix
1. **Introduce locale in userWorkspace entity**. The locale is stored on
workspaceMember in each postgres workspaceSchema (workspace_xxx) which
is the alter ego of userWorkspace in postgres core schema. Note that we
can't store it in user as a user can be part of multiple workspaces (the
locale already there must be seen as a default for this user), and we
cannot rely on workspaceMember as we would need to query the
workspaceSchema in the authentication layer which we want to avoid for
performance reasons.
2. During request hydration from token (containing the userWorkspaceId),
we fetch the userWorkspace and store it in the Request (this impact both
AuthContext and Request interface)
3. Leverage userWorkspace.locale in the useCachedMetadata hook
## Additional notes
There is no need to change the way we store and retrieve the
object-metadata-maps object itself which is different from the graphql
layer cache. object-metadadata-maps are not localized
We must separate the concept of hydratation which happens at the request
level (take the token and pass auth/user context), from the concept of
authorization which happens at the query/endpoint/mutation level.
Previously, hydratation exemption happened at the operation name level
which is not correct because the operation name is meaningless and
optional. Still this gave an impression of security by enforcing a
blacklist. So in this PR we introduce linting rule that aim to achieve a
similar behavior, now every api method has to have a guard. That way if
and endpoint is not protected by AuthUserGuard or AuthWorspaceGuard,
then it has to be stated explicitly next to its code.
---------
Co-authored-by: Charles Bochet <charles@twenty.com>
Follow-up on https://github.com/twentyhq/twenty/pull/12007
In this PR
- adding a filter on HttpExceptionHandlerService to filter out 4xx
errors from driver handling (as we do for graphQL errors: see
useGraphQLErrorHandler hook - only filteredIssues are sent to`
exceptionHandlerService.captureExceptions()`.)
- grouping together more missing metadata issues
- attempting to use error codes as issues names in sentry to improve UI;
for now it says "Error" all the time
# Introduction
Added a no-explicit-any rule to the twenty-server, not applicable to
tests and integration tests folder
Related to https://github.com/twentyhq/core-team-issues/issues/975
Discussed with Charles
## In case of conflicts
Until this is approved I won't rebased and handle conflict, just need to
drop two latest commits and re run the scripts etc
## Legacy
We decided not to handle the existing lint error occurrences and
programmatically ignored them through a disable next line rule comment
## Open question
We might wanna activate the
[no-explicit-any](https://typescript-eslint.io/rules/no-explicit-any/)
`ignoreRestArgs` for our use case ?
```
ignoreRestArgs?: boolean;
```
---------
Co-authored-by: etiennejouan <jouan.etienne@gmail.com>
# 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 !
In this PR
- introducing roles module to separate roles logic (assign a Role, get a
workspace's roles etc.) from permission logic (check if a user has a
permission)
- Introduces getRoles endpoint to fetch a workspace's roles
- introduces the first permission check: getRoles in only accessible to
users with permission on ROLE setting. Implemented
validatesUserHasWorkspaceSettingPermissionOrThrow
# 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
# This PR
- Addressing #3644
- Migrates the `DELETE /rest/*` endpoint to use TwentyORM
- Factorizes common middleware logic into a common module
---------
Co-authored-by: martmull <martmull@hotmail.fr>
