## Context
We recently introduced the new twenty ORM and used it in the update
methods in the query runner.
Initially we were using pg_graphql to fetch the record before updating
it allowing us to compare the before and the after and create a diff.
This diff is then used for the timeline activity creation. Now,
twentyORM is doing the fetch and pg_graphql is still doing the update
and their responses are not exactly the same, which means the diff is
not working as intended (e.g date types were always in the diff due to
one being in Date format and the other as a string)
This PR introduces a updatedFields property to the update event which
comes from the input. This is not ideal as this won't work for API users
that send the whole payload but will be sufficient enough for our FE
that only sends modified fields. We then compare only those fields in
the diff.
## Context
An object should always have a labelIdentifier (would be its primary key
at least). If the associated field is deleted by a user, it will break
the app. Ideally we should handle that on the DB level but we don't have
a FK for this column yet.
In the meantime I'm adding the validation check in the backend, note
that this is already handle on the FE side since the "archive/delete"
buttons don't appear for such fields so you need to reassign it to
another field first which is the desired behaviour.
## Context
LabelIdentifier and ImageIdentifier are metadata info attached to
objectMetadata that are used to display a record in a more readable way.
Those columns point to existing fields that are part of the object.
For example, for a relation picker of a person, we will show a record
using the "name" labelIdentifier and the "avatarUrl" imageIdentifier.
<img width="215" alt="Screenshot 2024-07-11 at 18 45 51"
src="https://github.com/twentyhq/twenty/assets/1834158/488f8294-0d7c-4209-b763-2499716ef29d">
Currently, the FE has a specific logic for company and people objects
and we have a way to update this value via the API for custom objects,
but the code is not flexible enough to change other standard objects.
This PR updates the WorkspaceEntity API so we can now provide the
labelIdentifier and imageIdentifier in the WorkspaceEntity decorator.
Example:
```typescript
@WorkspaceEntity({
standardId: STANDARD_OBJECT_IDS.activity,
namePlural: 'activities',
labelSingular: 'Activity',
labelPlural: 'Activities',
description: 'An activity',
icon: 'IconCheckbox',
labelIdentifierStandardId: ACTIVITY_STANDARD_FIELD_IDS.title,
})
@WorkspaceIsSystem()
export class ActivityWorkspaceEntity extends BaseWorkspaceEntity {
@WorkspaceField({
standardId: ACTIVITY_STANDARD_FIELD_IDS.title,
type: FieldMetadataType.TEXT,
label: 'Title',
description: 'Activity title',
icon: 'IconNotes',
})
title: string;
...
```
## Context
We've created a yoga (gql server) hook that catches requests and cache
them when needed. In practice we use it on the "objects" query because
this is often queried on the FE and it should never return something
different unless the schema has been intentionally changed by the user
when editing their data model (updating objects, fields, etc).
The issue here is we always cache the response regardless of its result,
even when it fails. This PR fixes that behaviour by only caching the
query response if it is successful.
I'm also fixing the cache key because the signature let users put
multiple operations and the cache key was not taking this into account
(we always use it on only one operation but we might have issues in the
future because another operation response could have erased the cached
response of another). Now the cache key contains the name of the
operation as well.
## Test
tested locally by manually throwing an error in the JWT auth guard
## Context
- Adding RATING sort and filter capabilities.
- Fixing isEmpty/isNotEmpty filters
- Fixing combined view filters so it combines filters per field metadata
and not per filter id. This is more a product question but to me it does
not make sense to apply multiples filters on the same field IF the
operations is wrapped in a AND. If at some point we want to put a OR
instead then that would make more sense
We call convertExceptionToGraphQLError in the exception handler for http
exceptions but we don't take into account those that already are
graphqlErrors and because of that the logic of convertExceptionToGraphql
is to fallback to a 500.
Now if the exception is a BaseGraphqlError (custom graphql error we
throw in the code), we throw them directly.
BEFORE
<img width="957" alt="Screenshot 2024-07-12 at 15 33 03"
src="https://github.com/user-attachments/assets/22ddae13-4996-4ad3-8f86-dd17c2922ca8">
AFTER
<img width="923" alt="Screenshot 2024-07-12 at 15 32 01"
src="https://github.com/user-attachments/assets/d3d6db93-6d28-495c-a4b4-ba4e47d45abd">
---------
Co-authored-by: Charles Bochet <charles@twenty.com>
Add a new command to delete objectMetadataId fieldMetadata that have a
wrong standard-id. This is because we have fixed the missing
objectMetadataId column but one already exists in the fieldMetadataId
with the wrong table. We should run this command before run the
sync-metadata.
Introduced a new module and command to run all the command associated
with the upgrade to 0.22. Not exactly sure with this structure but
ideally we would like to have only 1 command for version upgrades so
this is a first step.
## Context
We want to add an index on our foreign keys since PG does not do it for
us. An index can sometimes be expensive and not always meaningful
depending on different usages but in our case we decided to apply an
index for every foreign keys.
```typescript
@WorkspaceIndex()
@WorkspaceJoinColumn('author')
authorId: string;
```
This syntax is valid but since we want to apply it to every join column
I've decided to update the code of WorkspaceJoinColumn so it properly
registers a new index at the same time which is less error-prone.
Note: We had a bug on index name generation since postgres index names
are unique per schema and not table, the object metadata id (hashed) has
been added to the formula that generates the name of the index
## Test
Sync metadata. We have 45 join columns as of today per workspace, we
should see 45 rows inside IndexMetadata table
Insert inside AuditLog table are all failing due to objectMetadataId
column missing.
The FieldMetadata was sharing the same standard-id with another one
(objectName) so it was skipped during the comparison step of the
sync-metadata.
Running a sync-metadata again should fix this issue. Note that this
column is non-nullable so if the table contains existing records, it
will fail. However, since the insert was failing I'm assuming the table
is empty anyway.
## Context
Rating fields were not editable on the show page and kanban view when
they were null, this is because we don't have a way to leave the empty
state for fields that are editModeContentOnly.
~~This is actually an issue for bool fields (which is the other field
type that has editModeContentOnly) as well but they have default values
can't go be edited to NULL so it's not visible.~~
Actually let's fix bool, this could happen too
Hovering over "Empty" will now show the RatingField edit mode.
I'm not 100% sure about this solution though, we could also make this
behaviour on click? I preferred over since this is the behaviour on the
table view 🤔
## Test
https://github.com/twentyhq/twenty/assets/1834158/6825b5c3-2c62-41f2-8e03-343bc0e895e2
## Context
Our Flexible Schema engine dynamically generates entities/tables/APIs
for us but was not flexible enough to build indexes in the DB. With more
and more features involving heavy queries such as Messaging, we are now
adding a new WorkspaceIndex() decorator for our standard objects (will
come later for custom objects). This decorator will give enough
information to the workspace sync metadata manager to generate the
proper migrations that will create or drop indexes on demand.
To be aligned with the rest of the engine, we are adding 2 new tables:
IndexMetadata and IndexFieldMetadata, that will store the info of our
indexes.
## Implementation
```typescript
@WorkspaceEntity({
standardId: STANDARD_OBJECT_IDS.person,
namePlural: 'people',
labelSingular: 'Person',
labelPlural: 'People',
description: 'A person',
icon: 'IconUser',
})
export class PersonWorkspaceEntity extends BaseWorkspaceEntity {
@WorkspaceField({
standardId: PERSON_STANDARD_FIELD_IDS.email,
type: FieldMetadataType.EMAIL,
label: 'Email',
description: 'Contact’s Email',
icon: 'IconMail',
})
@WorkspaceIndex()
email: string;
```
By simply adding the WorkspaceIndex decorator, sync-metadata command
will create a new index for that column.
We can also add composite indexes, note that the order is important for
PSQL.
```typescript
@WorkspaceEntity({
standardId: STANDARD_OBJECT_IDS.person,
namePlural: 'people',
labelSingular: 'Person',
labelPlural: 'People',
description: 'A person',
icon: 'IconUser',
})
@WorkspaceIndex(['phone', 'email'])
export class PersonWorkspaceEntity extends BaseWorkspaceEntity {
```
Currently composite fields and relation fields are not handled by
@WorkspaceIndex() and you will need to use this notation instead
```typescript
@WorkspaceIndex(['companyId', 'nameFirstName'])
export class PersonWorkspaceEntity extends BaseWorkspaceEntity {
```
<img width="700" alt="Screenshot 2024-06-21 at 15 15 45"
src="https://github.com/twentyhq/twenty/assets/1834158/ac6da1d9-d315-40a4-9ba6-6ab9ae4709d4">
Next step: We might need to implement more complex index expressions,
this is why we have an expression column in IndexMetadata.
What I had in mind for the decorator, still open to discussion
```typescript
@WorkspaceIndex(['nameFirstName', 'nameLastName'], { expression: "$1 || ' ' || $2"})
export class PersonWorkspaceEntity extends BaseWorkspaceEntity {
```
---------
Co-authored-by: Charles Bochet <charles@twenty.com>
## Context
Fixing `setIsFocused is not a function` and the fact that edit buttons
were not showing up anymore.
A new FieldFocusContextProvider has been introduced and added to
RecordInlineCell but not ActivityTargetsInlineCell. This should fix the
issue.
<img width="523" alt="Screenshot 2024-06-05 at 17 42 07"
src="https://github.com/twentyhq/twenty/assets/1834158/1c1f919e-3829-4e40-b573-3b1b75b7c16f">
Our exception handler has to filter out some errors/exceptions so they
are not caught by the ExceptionHandlerDriver (Logged or Sentry for
example). This is done for Http errors in the range of 4xx and also
makes sure they are converted back to Graphql validation errors.
However, graphql validation errors that are already managed by Yoga
(with Schema validation) should also be filtered out, this PR should fix
that behaviour
## Context
Since pg_graphql does not return specific error/exception, we have to
map the error message and throw validation errors when needed
This PR adds a check on unicity constraint error returned by pg_graphql
when we are trying to insert duplicate records and returns a 400 instead
of being handled by the exceptionHandler as a 500.
## Context
Yoga can catch its own errors and we don't want to convert them again.
Moreover those errors don't have an "originalError" property and should
be schema related only (400 validation) so we only want to send them
back to the API caller without going through the exception handler.
Also fixed an issue in the createMany which was throwing a 500 when id
was missing from the creation payload. It seems the FE is always sending
an ID but it should actually be optional since the DB can generate one.
This is a regression from the new UUID validation introduced a few weeks
ago.
## Context
JobsModule is hard to maintain because we provide all the jobs there,
including their dependencies. This PR aims to split jobs in dedicated
modules.
## Context
Because creating an object in metadata also generates a graphql type and
because graphql does not allow 2 types with the same name, we have to
manage a list of reserved keywords that can't be used as object names.
Currently we were maintaining a list of the core objects but we also
have to introduce composite fields that are also generated as gql types.
## Context
Positions are used within a view to display and sort the different
records of standard/custom object.
When we add a new record and want to put it before the existing first
record, we have to use float values to insert them in the DB and respect
the desired order. We are adding a new command that can be executed to
flatten those positions.
---------
Co-authored-by: bosiraphael <raphael.bosi@gmail.com>
## Context
There is no calendarChannel syncStatus column compared to the
messageChannel table. In the meantime, we are trying to infer its status
based on the fact that the connection hasn't failed and the sync is
enabled
Various fixes
- Remote objects are read-only for now, we already hide and block most
of the write actions but the button that allows you to add a new record
in an empty collection was still visible.
- CreatedAt is not mandatory on remote objects (at least for now) so it
was breaking the show page, it now checks if createdAt exists and is not
null before trying to display the human readable format `Added x days
ago`
- The filters are overwritten in query-runner-args.factory.ts to handle
NUMBER field type, this was only working with filters like
```
{
"id": {
"in": [
1
]
}
```
but not with more depth such as
```
"and": [
{},
{
"id": {
"in": [
1
]
}
}
]
```
- Fixes CREATE FOREIGN TABLE raw query which was missing ",".
## Context
We recently introduced a change that now throws a 401 if the token is
invalid or expired.
The first implementation is using an allow list and 'IntrospectionQuery'
was missing so the playground was broken.
The check has been updated and we now only check the excludedOperations
list if a token is not present. This is because some operations can be
both used as loggedIn and loggedOut so we want to validate the token for
those sometimes (and set the workspace, user, cache version, etc). Still
not a very clean solution imho.
## Context
We have a non-nullable constraint on authorId in attachments and
documents, until we have soft-deletion we need to handle deletion of
workspace-members and their attachments/documents.
This PR introduces pre-hooks to deleteOne/deleteMany
This is called when a user deletes a workspace-member from the members
page
Next: needs to be done on user level as well. This is called when users
try to delete their own accounts. I've seen other issues such as
re-creating a user with a previously used email failing.
## Context
Those settings are not implemented yet, we would like to move them to a
different page as well.
In the meantime, we are hiding them since we plan to launch calendar in
the next release and this won't be implemented before.
We will implement it in this
https://github.com/twentyhq/twenty/issues/5140
## Context
Currently, this middleware validates the token and stores the user,
workspace and cacheversion in the request object.
It only does so when a token is provided and ignores the middleware
logic if not. If the token is invalid or expired, the exception is
swallowed.
This PR removes the try/catch and adds an allowlist to skip the token
validation for operations executed while not signed-in.
I don't know a better way to do that with Nestjs. We can't easily add
the middleware per resolver without refactoring the flexible schema
engine so I'm doing it the other way around.
Fixes https://github.com/twentyhq/twenty/issues/5224
## Context
Currently we have an unicity constraint in the DB but we don't return a
clear error to the frontend before reaching the DB (which then throws a
500). This PR adds a validation check similar to what we have with field
creation
Now that we have persistent cache for schemas, we want to be able to
reset its state when users run the database:reset db otherwise schemas
won't be synced with the new DB state.
Note: In an upcoming PR, we want to be able to invalidate the cache on a
workspace level when we change the metadata schema through twenty
version upgrade
## Context
Messaging and calendar cron jobs are only working for workspace that
have sub status different than incomplete, this is because currently
this is the simplest way to know if a user is onboarded. This should not
be the source of truth and this will be updated in a later version. In
the meantime, to make self-hosting easier, we are adding an extra check
on IS_BILLING_ENABLED env var since sub status is not relevant for
people not using billing.
A search bar has been introduced in the filter dropdown menu however we
don't want to apply debounce since the search has no side-effect and is
purely FE (it's not querying the DB compared to other search bars). Also
adding autofocus on the search bar when the dropdown is open.
## Context
@lucasbordeau introduced a new Yoga plugin that allows us to cache our
requests (👏), see https://github.com/twentyhq/twenty/pull/5189
I'm simply updating the implementation to allow us to use different
cache storage types such as redis
Also adding a check so it does not use cache for other operations than
ObjectMetadataItems
## Test
locally, first call takes 340ms, 2nd takes 30ms with 'redis' and 13ms
with 'memory'
## Context
We recently introduced this verification but we didn't take into account
self-hosting that might not use billing.
## Test
tested locally with
- new workspace and new account
- existing workspace with new account and billing not enabled and status
incomplete => OK
- existing workspace with new account and billing enabled and status
incomplete => NOK
- existing workspace with new account and billing enabled and status
active => OK
## Context
- Rename remaining V2 services.
- Delete messages in DB when gmail history tells us they've been
deleted. I removed the logic where we store those in a cache since it's
a bit overkill because we don't need to query gmail and can use those
ids directly. The strategy is to delete the message channel message
association of the current channel, not the message or the thread since
they can still be linked to other channels. However, we will need to
call the threadCleaner service on the workspace to remove orphan
threads/non-associated messages.
Note: deletion for full-sync is a bit tricky because we need the full
list of message ids to compare with the DB and make sure we don't
over-delete. Currently, to keep memory, we don't have a variable that
holds all ids as we flush it after each page. Easier solution would be
to wipe everything before each full sync but it's probably not great for
the user experience if they are currently manipulating messages since
full-sync can happen without a user intervention (if a partial sync
fails due to historyId being invalidated by google for some reason)
## Context
The full-sync job was enqueued within a transaction, which means it
could be executed before the transaction was commit and
connected-account was not created yet.
This PR re-arrange the code a bit to avoid this
cc @bosiraphael thx for flagging this!
## Context
Recent PR introduced a verifyTransientToken inside the
GoogleAPIsProviderEnabledGuard guard. This is used to extract the
workspaceId from the token. This is working fine for the first call sent
to google however the callback is calling the same guard which is
causing an issue because the transientToken is missing from the
callback.
Imho, the same guard shouldn't be used by the callback but for the time
being I'm adding a check to prevent using feature flag when
transientToken is absent. In fact, it is present in the request but not
in the same key. Because the scope is only relevant for the first call,
I'm simply adding a check there.
## Context
Currently the calendar scope is bound to an env variable. We want to
rollout this feature to some users so this PR adds a check on the
existing IS_CALENDAR_ENABLED flag
## Context
With the addition of cronjobs, the app is building a lot of jobs and
stores them indefinitely. There is no real point to keep all of them in
the queue once they have been processed (completed or failed) so we are
adding a new default option to the bull-mq driver
## Implementation
See bull-mq JobsOption doc
```typescript
/**
* If true, removes the job when it successfully completes
* When given a number, it specifies the maximum amount of
* jobs to keep, or you can provide an object specifying max
* age and/or count to keep. It overrides whatever setting is used in the worker.
* Default behavior is to keep the job in the completed set.
*/
removeOnComplete?: boolean | number | KeepJobs;
/**
* If true, removes the job when it fails after all attempts.
* When given a number, it specifies the maximum amount of
* jobs to keep, or you can provide an object specifying max
* age and/or count to keep. It overrides whatever setting is used in the worker.
* Default behavior is to keep the job in the failed set.
*/
removeOnFail?: boolean | number | KeepJobs;
```
removeOnFail should be a bit higher since they are the ones we are most
likely looking at when needed.
This PR introduces a new folder structure for business modules.
Cron commands and jobs are now stored within the same module/folder at
the root of the business module
e.g: /modules/messaging/crons/commands instead of
/modules/messaging/commands/crons
Patterns are now inside their own cron-command files since they don't
need to be exported
Ideally cronJobs and cronCommands should have their logic within the
same class but it's a bit harder than expected due to how commanderjs
and our worker need both some class heritage check, hence the first
approach is to move them in the same folder
Also Messaging fullsync/partialsync V2 has been dropped since this is
the only used version => Breaking change for ongoing jobs and crons.
Jobs can be dropped but we will need to re-run our crons (only
cron:messaging:gmail-fetch-messages-from-cache)
- AuthFailedAt is set when a refreshToken is not valid and an
accessToken can't be generated, meaning it will need a manual action
from the user to provide a new refresh token.
- Calendar/messaging jobs should not be executed if authFailedAt is not
null.
## Context
Calendar scope was too broad, this PR updates it to events only.
Also changing "Cannot connect Google account to demo workspace" error to
a 404 to avoid having a 500 for something expected
## Context
We are now removing Messaging V2 feature flag to use it everywhere.
## Implementation
- renaming FetchWorkspaceMessagesCommandsModule to
MessagingCommandModule to make it more generic since it it hosts all
commands related to the messaging module
- creating a crons folder inside commands and jobs crons should be named
with xxx.cron.command.ts instead of xxx.command.ts. Same for jobs, jobs
should be named with xxx.cron.job.ts. In a future PR we should make sure
those CronJobs implement a CronJob interface since it's a bit different
(a CronJob does not contain a payload compared to a Job)
- Cron commands have been renamed to "cron:$module:command" so
`fetch-all-workspaces-messages-from-cache:cron:start` has been renamed
to `cron:messaging:gmail-fetch-messages-from-cache`. Also having to
create a command to stop the cron is a bit painful to maintain so I
removed them for now, this can be easily done manually with pg-boss or
bull-mq
- Removing full-sync and partial-sync commands as they were there for
testing only, we might put them back at some point but we will have to
adapt the code anyway.
- Feature flag has been removed from the MessageChannel standard object
to make sure those new columns are created during the next sync-metadata
## Context
Calendar tables are behind a featureFlag, they do not exist if the
feature flag is off which means we should not use them for the same
reason. I'm adding a check on the featureFlag before calling the
repository.
```
Error executing raw query for workspace 20202020-1c25-4d02-bf25-6aeccf7ea419: relation "workspace_1wgvd1injqtife6y4rvfbu3h5.calendarEventParticipant" does not exist
```
## Test
locally with and without featureflag
## Context
When running a command, the process should end normally however it stays
hanging due to the open connection with redis client (when
CACHE_STORAGE_TYPE=redis)
This PR adds the necessary logic to gracefully close the connection once
the module is destroyed. Thanks to that, the command process now
properly ends once executed.
## Context
SyncExternalId should be renamed because this won't always represent an
id. For example, microsoft API does not use ids but dates for their
sync. Also we think external is a bit redundant so we are removing it.
Note: this field is not used by the front-end (and will probably never
be)
## Context
A new ADDRESS field type has been introduced and the company object has
been updated to use this new type however this introduced a few
regressions.
The good strategy would be to introduce a new field and rename the old
one.
This PR revert that change to fix the issue.
* rename database services to repository
* refactor more repositories
* more refactoring
* followup
* remove unused imports
* fix
* fix
* Fix calendar listener being called when flag is off
* remove folders
* Fix delete with no result should not throw
* add logs
* Delete packages/twenty-server/src/database/typeorm/metadata/migrations/1708442904165-addOnDeleteActionToRelationMetadata.ts
Weiko