## Context
Following https://github.com/twentyhq/twenty/pull/16399
Now using the new global orm manager everywhere and returning a
GlobalDatasource/WorkspaceDatasource based on a feature flag.
This means we now need to wrap all our ORM calls within
executeInWorkspaceContext callback (at least for now) so the global
datasource can dynamically hydrate its context via the new store (the
global datasource does not store anything related to workspaces as it is
now a unique singleton). If feature flag is off it still uses local data
stored in the workspace datasource.
In this PR, I'm solving several issues:
1) We were not checking if the currentWorkspaceMember was owning the
message in the thread. It kind of worked before because the case of
shared threads (with shared threads visibility restriction) was not
happening that often. It seems that the bug has always been there
2) Re-implement orphan messages and threads deletion on messageChannel
deletion. We used to brutally look for all orphans, we disabled it last
week because it was too heavy on db. I've re-implemented it more
carefully and "surgically"
3) Gmail sync was not handling folder synced correctly. It was
leveraging labelIds which it shouldn't do (this is a AND AND parameter)
in full sync
4) Added a command to clean orphan message threads manually if needed.
Usually this is done when you remove a messageChannel, or change
blocklist rules but it can be useful to have it to debug
First and main step of
https://github.com/twentyhq/core-team-issues/issues/747
We are implementing a permission check layer in our custom
WorkspaceEntityManager by overriding all the db-executing methods (this
PR only overrides some as a POC, the rest will be done in the next PR).
Our custom repositories call entity managers under the hood to interact
with the db so this solves the repositories case too.
This is still behind the feature flag IsPermissionsV2Enabled.
In the next PR
- finish overriding all the methods required in WorkspaceEntityManager
- add tests
Closes#6657
- Fix listeners
- Refactor jobs to take array of events
- Fix calendar events and messages deletion
---------
Co-authored-by: Charles Bochet <charles@twenty.com>
This PR fix an issue with the `IsNull()` find operator applied on
one-to-many relation, this one is not supported by TypeORM.
We can instead filter by an empty array to retrieve object with empty
relations.
In this PR, I'm refactoring the messaging module into smaller pieces
that have **ONE** responsibility: import messages, clean messages,
handle message participant creation, instead of having ~30 modules (1
per service, jobs, cron, ...). This is mandatory to start introducing
drivers (gmails, office365, ...) IMO. It is too difficult to enforce
common interfaces as we have too many interfaces (30 modules...). All
modules should not be exposed
Right now, we have services that are almost functions:
do-that-and-this.service.ts / do-that-and-this.module.ts
I believe we should have something more organized at a high level and it
does not matter that much if we have a bit of code duplicates.
Note that the proposal is not fully implemented in the current PR that
has only focused on messaging folder (biggest part)
Here is the high level proposal:
- connected-account: token-refresher
- blocklist
- messaging: message-importer, message-cleaner, message-participants,
... (right now I'm keeping a big messaging-common but this will
disappear see below)
- calendar: calendar-importer, calendar-cleaner, ...
Consequences:
1) It's OK to re-implement several times some things. Example:
- error handling in connected-account, messaging, and calendar instead
of trying to unify. They are actually different error handling. The only
things that might be in common is the GmailError => CommonError parsing
and I'm not even sure it makes a lot of sense as these 3 apis might have
different format actually
- auto-creation. Calendar and Messaging could actually have different
rules
2) **We should not have circular dependencies:**
- I believe this was the reason why we had so many modules, to be able
to cherry pick the one we wanted to avoid circular deps. This is not the
right approach IMO, we need architect the whole messaging by defining
high level blocks that won't have circular dependencies by design. If we
encounter one, we should rethink and break the block in a way that makes
sense.
- ex: connected-account.resolver is not in the same module as
token-refresher. ==> connected-account.resolver => message-importer (as
we trigger full sync job when we connect an account) => token-refresher
(as we refresh token on message import).
connected-account.resolver and token-refresher both in connected-account
folder but should be in different modules. Otherwise it's a circular
dependency. It does not mean that we should create 1 module per service
as it was done before
In a nutshell: The code needs to be thought in term of reponsibilities
and in a way that enforce high level interfaces (and avoid circular
dependencies)
Bonus: As you can see, this code is also removing a lot of code because
of the removal of many .module.ts (also because I'm removing the sync
scripts v2 feature flag end removing old code)
Bonus: I have prefixed services name with Messaging to improve dev xp.
GmailErrorHandler could be different between MessagingGmailErrorHandler
and CalendarGmailErrorHandler for instance
Weiko