Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data changes to prepare for workspaceMember page #10439

Merged
merged 7 commits into from
Feb 24, 2025

Conversation

FelixMalfait
Copy link
Member

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

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

This PR prepares the backend for a dedicated workspace member page by renaming fields and adjusting relationships to avoid conflicts with morph relationships.

  • Renamed workspaceMember to forWorkspaceMember in /packages/twenty-server/src/modules/favorite/standard-objects/favorite.workspace-entity.ts to prevent conflicts
  • Added position field with ID 20202020-1810-4591-a93c-d0df97dca843 to WORKSPACE_MEMBER_STANDARD_FIELD_IDS in /packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/constants/standard-field-ids.ts
  • Added @WorkspaceIsSystem() decorators to protect fields in WorkspaceMemberWorkspaceEntity from unauthorized modifications
  • Updated all favorite-related hooks and components to use forWorkspaceMemberId instead of workspaceMemberId for consistency

16 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile

@@ -82,7 +82,7 @@ export const sortedFavorites = [
labelIdentifier: ' ',
link: '/object/person/3',
objectNameSingular: 'person',
workspaceMemberId: '1',
forWorWorkspaceMemberId: '1',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: Typo in field name 'forWorWorkspaceMemberId' - should be 'forWorkspaceMemberId'

Suggested change
forWorWorkspaceMemberId: '1',
forWorkspaceMemberId: '1',

@@ -72,7 +72,7 @@ export const FavoriteFolderPickerEffect = ({
const checkedFolderIds = favorites
.filter(
(favorite) =>
favorite.recordId === targetId && favorite.workspaceMemberId,
favorite.recordId === targetId && favorite.forWorkspaceMemberId,
)
.map((favorite) => favorite.favoriteFolderId || 'no-folder');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider handling the case where favoriteFolderId is undefined more explicitly rather than defaulting to 'no-folder' string literal

FelixMalfait and others added 5 commits February 24, 2025 14:30
…bjects/workspace-member.workspace-entity.ts

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Copy link
Collaborator

@ijreilly ijreilly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works for me !
I wonder if it is something we should take into account in the frame of permissions. it seems to be a good (future) usecase for row-level permissions: a workspace member can only update their own favorite. Very edgy for now and not worth the effort in my opinion but to keep in mind if we introduce other objects with this concept of "row-owner". @Weiko

@FelixMalfait FelixMalfait merged commit cbd4d98 into main Feb 24, 2025
50 checks passed
@FelixMalfait FelixMalfait deleted the prepare-workspace-member-page branch February 24, 2025 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants