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

[REFACTOR][FRONT]: Remove objectMetadata and fieldMetadata sluggification #9441

Merged
merged 7 commits into from
Jan 8, 2025

Conversation

prastoin
Copy link
Contributor

@prastoin prastoin commented Jan 7, 2025

Introduction

For motivations and context please have a look to #9394 whom this PR results from.
In this pull-request we remove any metadataField and objectMetadata sluggification. We directly consume objectMetadata.namePlural and metadataField.name, it seems like that historically the consumed metadataField.name are we sure that we wanna change this behavior ?

Notes

Unless I'm mistaken by reverting the kebabcase url formatting we might be creating deadlinks that user could have save beforehand => Discussed with Charles said it's controlled risk.

@prastoin prastoin changed the title [REFACTORRefactor object field slugification [REFACTOR][FRONT]: Remove object field sluggification Jan 7, 2025
@prastoin prastoin marked this pull request as ready for review January 7, 2025 15:53
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 implements a significant refactoring to remove slugification (kebab-case formatting) across the codebase, directly using object and field names instead. Here are the key changes:

  • Removed getObjectSlug.ts and getFieldSlug.ts utility files and their associated tests
  • Replaced findObjectMetadataItemBySlug with findObjectMetadataItemByNamePlural in metadata hooks
  • Updated URL construction to use raw objectMetadataItem.namePlural and fieldMetadataItem.name values
  • Modified test files to reflect new naming conventions and removed slug-related tests
  • Renamed methods and variables across components to align with the new direct naming approach

The main risk is potential dead links for users who bookmarked URLs using the old kebab-case format, though this has been acknowledged as a controlled risk by the team.

💡 (2/5) Greptile learns from your feedback when you react with 👍/👎!

19 file(s) reviewed, 15 comment(s)
Edit PR Review Bot Settings | Greptile

@prastoin prastoin marked this pull request as draft January 7, 2025 16:09
@prastoin prastoin changed the title [REFACTOR][FRONT]: Remove object field sluggification [REFACTOR][FRONT]: Remove objectMetadata and fieldMetadata sluggification Jan 7, 2025
@prastoin prastoin marked this pull request as ready for review January 7, 2025 16:42
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

(updates since last review)

This PR continues the refactoring work to remove slugification, with significant changes to URL handling and state management. Here are the key updates:

  • Replaced updatedObjectSlugState with new updatedObjectNamePluralState for managing raw object names
  • Updated dropdown components to use objectNamePlural instead of objectSlug in URL construction
  • Modified settings page components to handle raw field names and object names in navigation
  • Changed route parameters from :objectSlug/:fieldSlug to :objectNamePlural/:fieldName across stories and tests
  • Removed kebab-case formatting from all URL paths in SettingsPath enum

The changes maintain consistency with the new direct naming approach while acknowledging the controlled risk of breaking existing bookmarked URLs.

18 file(s) reviewed, 12 comment(s)
Edit PR Review Bot Settings | Greptile

@lucasbordeau lucasbordeau self-assigned this Jan 8, 2025
@lucasbordeau lucasbordeau self-requested a review January 8, 2025 09:51
Copy link
Contributor

@lucasbordeau lucasbordeau left a comment

Choose a reason for hiding this comment

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

Nice refactor

@prastoin prastoin requested a review from lucasbordeau January 8, 2025 10:17
@thomtrp thomtrp merged commit aa0d854 into main Jan 8, 2025
23 checks passed
@thomtrp thomtrp deleted the refactor-object-field-slugification branch January 8, 2025 10:31
Copy link

github-actions bot commented Jan 8, 2025

Fails
🚫

node failed.

Log

�[31mError: �[39m SyntaxError: Unexpected token C in JSON at position 0
    at JSON.parse (<anonymous>)
�[90m    at parseJSONFromBytes (node:internal/deps/undici/undici:5584:19)�[39m
�[90m    at successSteps (node:internal/deps/undici/undici:5555:27)�[39m
�[90m    at fullyReadBody (node:internal/deps/undici/undici:1665:9)�[39m
�[90m    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)�[39m
�[90m    at async specConsumeBody (node:internal/deps/undici/undici:5564:7)�[39m
danger-results://tmp/danger-results-ef4bbc07.json

Generated by 🚫 dangerJS against 42f0008

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.

3 participants