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

Sort Metadata Fields by Custom Status and Creation Date #3254

Merged
merged 4 commits into from
Jan 10, 2024

Conversation

Jeong-Min-Cho
Copy link
Contributor

@Jeong-Min-Cho Jeong-Min-Cho commented Jan 5, 2024

Summary

This pull request closes #3253 and introduces a change in the way we display metadata fields in objects such as People, Opportunities, Companies. With this change, the fields are sorted first by their isCustom status (standard fields first, then custom fields), and then by their creationDate (older fields first).

Changes:

  1. Created a new function sortFieldMetadataItem in utils.ts that takes two fields and returns a sorting score based on the isCustom status and creationDate.

  2. Used the sortFieldMetadataItem function to sort activeMetadataFields and disabledMetadataFields before they are displayed in SettingsObjectDetails.tsx

  3. Created Jest tests for sortFieldMetadataItem utility file.

Test

  1. Navigate to the settings page of an object (http://localhost:3001/settings/objects).
  2. Add a new custom field and verify that it appears last in the list of fields.
  3. Verify that both of the active and disabled fields are sorted first by Custom status (standard first, then custom), and then by Creation Date (older first).

Screenshots

Before

image

After

image

Checklist

  • Extract the sort fucntion to util.ts
  • Add Jest tests for it

Copy link
Member

@charlesBochet charlesBochet left a comment

Choose a reason for hiding this comment

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

Love it!
Could you extract this function to an util.ts and add jest tests on top of it?

I feel we are going to have regressions otherwise

@Jeong-Min-Cho
Copy link
Contributor Author

@charlesBochet
Gladly. I also don't want any regression from my pull request either. 😊

@Jeong-Min-Cho
Copy link
Contributor Author

@charlesBochet Hello again!
The changes are up now 🚀

Copy link
Member

@Weiko Weiko left a comment

Choose a reason for hiding this comment

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

LGTM @Jeong-Min-Cho, thanks for adding tests 💪

@Weiko Weiko merged commit a791d1f into twentyhq:main Jan 10, 2024
10 of 11 checks passed
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.

New fields should be sorted on objet detail page
4 participants