Fix remote object read-only + remove relations #4921
Conversation
thomtrp
force-pushed
the
tt-deactivate-relations-for-remote
branch
from
a86664b to
62dc378
Compare
thomtrp
force-pushed
the
tt-deactivate-relations-for-remote
branch
from
bd80931 to
15fd09f
Compare
| [download, progress], | ||
| ); | ||
|
|
||
| const baseActions = objectMetadataItem.isRemote |
There was a problem hiding this comment.
I don't think it's enough, we cannot add a favorite on remote object either. (it's below in the code).
I would update base action to only contain export
then I would append deletion and favorittes for nonRemotes
| onTab={handleTab} | ||
| /> | ||
| isReadOnly ? ( | ||
| <FieldDisplay /> |
There was a problem hiding this comment.
looks like a hack. I would pass isReadOnly to the recordTableContainer and prevent edit mode activation
| const isFieldInputOnly = useIsFieldInputOnly(); | ||
|
|
||
| const { openTableCell } = useOpenRecordTableCell(); | ||
|
|
||
| const handleClick = () => { | ||
| setSoftFocusOnCurrentCell(); | ||
|
|
||
| if (!isFieldInputOnly) { | ||
| if (!isFieldInputOnly && !isReadOnly) { |
There was a problem hiding this comment.
i would not check it here but within the openTableCell itself
| @@ -95,7 +98,7 @@ export const RecordTableCellSoftFocusMode = ({ | |||
| ); | |||
|
|
|||
| const handleClick = () => { | |||
| if (!isFieldInputOnly) { | |||
| if (!isFieldInputOnly && !isReadOnly) { | |||
There was a problem hiding this comment.
so you don't have to add it several times
| @@ -855,4 +868,14 @@ export class ObjectMetadataService extends TypeOrmQueryService<ObjectMetadataEnt | |||
|
|
|||
| return { favoriteObjectMetadata }; | |||
| } | |||
|
|
|||
| private async isRelationEnabledForRemoteObjects(workspaceId: string) { | |||
| const featureFlag = await this.featureFlagRepository.findOneBy({ | |||
There was a problem hiding this comment.
I'm surprise we don't have a generic helper in the backend isFeatureFlagEnabled(KEY) this logic of existence + boolean = true seems standard
There was a problem hiding this comment.
we don't. But I can do a refacto. I tried to make it a guard not so long ago but it was not working well. An util would be easy
| objectMetadataInput.workspaceId, | ||
| createdObjectMetadata, | ||
| ); | ||
| if (isCustom || isRelationEnabledForRemoteObjects) { |
There was a problem hiding this comment.
This looks suspicious: create object is necessarily either custom or remote. so I would actually create a function call: createObjectRelations. First line would be: if (! isRelationEnabledForRemoteObjects) { return }
| @@ -74,19 +74,6 @@ export const buildWorkspaceMigrationsForRemoteObject = async ( | |||
| } satisfies WorkspaceMigrationColumnCreate, | |||
| ], | |||
| }, | |||
| { | |||
| name: computeObjectTargetTable(activityTargetObjectMetadata), | |||
There was a problem hiding this comment.
so the featureflag is partially followed? we always remove from migrations?
There was a problem hiding this comment.
Nope this part is only cleaning duplicated code
thomtrp
force-pushed
the
tt-deactivate-relations-for-remote
branch
from
9bf8f68 to
d48b693
Compare
- Set `readOnly` boolean in table row context. Preventing updates and deletion - Show page is null for remote objects. No need for complicated design since this is temporary? - Relation creations are now behind a feature flag for remote objects - Refetch objects and views after syncing objects --------- Co-authored-by: Thomas Trompette <thomast@twenty.com>
readOnlyboolean in table row context. Preventing updates and deletion