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

add new @WorkspaceIsSearchable decorator + updates services + add migration command #10507

Merged
merged 6 commits into from
Feb 27, 2025

Conversation

etiennejouan
Copy link
Contributor

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 adds a new @WorkspaceIsSearchable decorator and isSearchable property to control which entities can be included in search results.

  • Added new isSearchable boolean column to objectMetadata table with migration file 1740478150675-addIsSearchableColumInObjectMetadataTable.ts
  • Created migration command 0-43-migrate-is-searchable-for-custom-object-metadata.command.ts to set isSearchable=true for existing custom objects
  • Simplified GlobalSearchService.filterObjectMetadataItems() to use the new isSearchable property instead of hardcoded object types
  • Applied @WorkspaceIsSearchable() decorator to standard entities (Person, Company, Note, Task, Opportunity)
  • Removed utility functions isObjectMetadataItemSearchable and isObjectMetadataItemSearchableInCombinedRequest in favor of direct property checks

38 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile

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.

Enregistrement.de.l.ecran.2025-02-27.a.14.16.36.mov

Just ran the upgrade command. maybe I need to activate something else ?

Update: seems like upgrade command is not enough, sync-metadata must be run too. Not sure how this is usually indicated to self hosters and ppl in charge of release

super(workspaceRepository, twentyORMGlobalManager);
}

async runMigrationCommandOnActiveWorkspaces(
Copy link
Collaborator

Choose a reason for hiding this comment

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

we now also maintain workspaces that are not active right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my command extends ActiveWorkspacesMigrationCommandRunner command which expects runMigrationCommandOnActiveWorkspaces method. It is called 'active' but it includes active and suspended workspaces - all workspaces who need to be maintained.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok thank you
maybe we could rename ActiveWorkspaces with MaintainedWorkspaces (or a better english term)?

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.

we still have the sync-metadata topic but i think you said it is being tackled !

@etiennejouan
Copy link
Contributor Author

Enregistrement.de.l.ecran.2025-02-27.a.14.16.36.mov
Just ran the upgrade command. maybe I need to activate something else ?

Update: seems like upgrade command is not enough, sync-metadata must be run too. Not sure how this is usually indicated to self hosters and ppl in charge of release

you're right, not working well. @prastoin is currently fixing on the upgrade command bug that should sync-metadata.

@etiennejouan etiennejouan merged commit 3954387 into main Feb 27, 2025
47 checks passed
@etiennejouan etiennejouan deleted the ej/345 branch February 27, 2025 12:57
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.

Add new WorkspaceIsSearchable decorator
2 participants