feat: migrate routing forms to routing #18161

VK-RED · 2024-12-13T11:20:46Z

What does this PR do?

Setup /routing and /routing/:slug page
Added redirect from /apps/routing-forms to /routing, (ex: /apps/routing-forms/forms -> /routing/forms)
Renamed "Routing Forms" to "Routing" and changed the icon to split
Fixes [CAL-4828] migrate /apps/routing-forms/forms to /routing #17955
Fixes CAL-4828

Screencast

Screencast.from.2024-12-13.16-40-23.webm

Mandatory Tasks (DO NOT REMOVE)

I have self-reviewed the code (A decent size PR without self-review might be rejected).
I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
I confirm automated tests are in place that prove my fix is effective or that my feature works.

vercel · 2024-12-13T11:20:51Z

@VK-RED is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

github-actions · 2024-12-13T11:20:58Z

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

No release type found in pull request title "Feat/migrate routing forms to routing". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

VK-RED · 2024-12-13T11:22:56Z

Also do we need to update this from Routing Forms to Routing ??

graphite-app · 2024-12-13T11:25:12Z

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (12/13/24)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add community label" took an action on this PR • (12/13/24)

1 label was added to this PR based on Keith Williams's automation.

apps/web/lib/routing/getServerSideProps.ts

apps/web/modules/routing/pages-view.tsx

packages/features/shell/navigation/Navigation.tsx

apps/web/lib/routing/getStaticProps.ts

Praashh

We also need test cases for this feature, @VK-RED can you please add ?

Praashh · 2024-12-13T13:31:16Z

Also do we need to update this from Routing Forms to Routing ??

no we don't need to update it.

VK-RED · 2024-12-13T14:14:24Z

Sure will add the test cases 👍

VK-RED · 2024-12-14T06:52:08Z

Updated the routing

/apps/routing-forms/forms -> /routing
/apps/routing-forms/:slug -> /routing/:slug

Screencast

Screencast.from.2024-12-14.12-17-32.webm

Praashh · 2024-12-18T05:08:59Z

apps/web/lib/routing/getStaticProps.ts

+
+  const isTemplate = appMeta.isTemplate;
+  const appDirname = path.join(isTemplate ? "templates" : "", appFromDb.dirName);
+  const README_PATH = path.join(process.cwd(), "..", "..", `packages/app-store/${appDirname}/DESCRIPTION.md`);


Hard-coded paths could cause issues in different environments, also there should be path validation.

Do you have any idea of how can we avoid hard coded Path, I can't think of it ??

we can probably store all path configurations in a centralized config file, and use that file

Praashh · 2024-12-18T05:10:00Z

apps/web/lib/routing/getStaticProps.ts

+  let source = "";
+
+  try {
+    source = fs.readFileSync(postFilePath).toString();


this could block the event loop, promise would the better choice to use here.

Changing it

Praashh · 2024-12-18T05:11:21Z

apps/web/lib/routing/getStaticProps.ts

+  try {
+    source = fs.readFileSync(postFilePath).toString();
+    source = source.replace(/{DESCRIPTION}/g, appMeta.description);
+  } catch (error) {
+    /* If the app doesn't have a README we fallback to the package description */
+    source = appMeta.description;
+  }


it catches all errors, not just file not-found which is bad.

Praashh · 2024-12-18T05:16:19Z

apps/web/lib/routing/getStaticProps.ts

+  let source = "";
+
+  try {
+    source = fs.readFileSync(postFilePath).toString();


toString() without encoding specification could cause issues with non UTF8 files

VK-RED · 2024-12-19T14:19:20Z

@Praashh The file you have mentioned changes was added Previously when I misunderstood /apps/routing-forms -> /routing . So the apps/web/lib/routing/getStaticProps.ts is no longer needed .

VK-RED added 6 commits December 13, 2024 16:11

feat: setup routing page

f13233e

feat: generate split icon

78a981f

fix: routing page link and icon

4dc0680

feat: setup redirect for /apps/routing-forms

5184d99

Merge branch 'main' into feat/migrate-routing-forms-to-routing

d230e39

chore: remove unsused variable and types

5f39fc5

graphite-app bot added the community Created by Linear-GitHub Sync label Dec 13, 2024

graphite-app bot requested a review from a team December 13, 2024 11:20

github-actions bot added Medium priority Created by Linear-GitHub Sync routing-forms area: routing forms, routing, forms ⚡ Quick Wins A collection of quick wins/quick fixes that are less than 30 minutes of work labels Dec 13, 2024

dosubot bot added the ✨ feature New feature or request label Dec 13, 2024

VK-RED changed the title ~~Feat/migrate routing forms to routing~~ feat: migrate routing forms to routing Dec 13, 2024

Praashh reviewed Dec 13, 2024

View reviewed changes

apps/web/lib/routing/getServerSideProps.ts Outdated Show resolved Hide resolved

Praashh reviewed Dec 13, 2024

View reviewed changes

apps/web/modules/routing/pages-view.tsx Outdated Show resolved Hide resolved

Praashh reviewed Dec 13, 2024

View reviewed changes

packages/features/shell/navigation/Navigation.tsx Outdated Show resolved Hide resolved

Praashh reviewed Dec 13, 2024

View reviewed changes

apps/web/lib/routing/getStaticProps.ts Outdated Show resolved Hide resolved

Praashh requested changes Dec 13, 2024

View reviewed changes

Praashh self-assigned this Dec 13, 2024

VK-RED added 5 commits December 13, 2024 20:13

refactor: add PageProps type as props type

9c92af1

refactor: remove logs

6d7a4dc

refactor: add getServerSideProps for forms page

b8a4f0d

feat: add getServerSideProps for routing forms pages

d1d6295

feat: add PagsProps type to props

05d1ef5

VK-RED added 7 commits December 14, 2024 08:20

feat: add forms to be the main page

84e102c

feat: add forms as default page on layout-handler

eae1da6

refactor: modify redirect for apps/routing-forms/forms

c3bc0b4

refactor: routing navigation link

67abefb

feat: add tests for redirect

f63d131

fix: routing page link

707c678

Merge branch 'main' into feat/migrate-routing-forms-to-routing

9070cc4

VK-RED requested a review from Praashh December 18, 2024 02:35

Praashh reviewed Dec 18, 2024

View reviewed changes

VK-RED added 2 commits December 19, 2024 19:44

chore: drop routing getStaticProps

b238dac

Merge branch 'main' into feat/migrate-routing-forms-to-routing

4fa27e8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: migrate routing forms to routing #18161

feat: migrate routing forms to routing #18161

VK-RED commented Dec 13, 2024

vercel bot commented Dec 13, 2024

github-actions bot commented Dec 13, 2024

VK-RED commented Dec 13, 2024

graphite-app bot commented Dec 13, 2024

Praashh left a comment

Praashh commented Dec 13, 2024

VK-RED commented Dec 13, 2024

VK-RED commented Dec 14, 2024

Praashh Dec 18, 2024

VK-RED Dec 18, 2024

Praashh Dec 18, 2024

Praashh Dec 18, 2024

VK-RED Dec 18, 2024

Praashh Dec 18, 2024

Praashh Dec 18, 2024

VK-RED commented Dec 19, 2024

feat: migrate routing forms to routing #18161

Are you sure you want to change the base?

feat: migrate routing forms to routing #18161

Conversation

VK-RED commented Dec 13, 2024

What does this PR do?

Screencast

Mandatory Tasks (DO NOT REMOVE)

vercel bot commented Dec 13, 2024

github-actions bot commented Dec 13, 2024

VK-RED commented Dec 13, 2024

graphite-app bot commented Dec 13, 2024

Graphite Automations

Praashh left a comment

Choose a reason for hiding this comment

Praashh commented Dec 13, 2024

VK-RED commented Dec 13, 2024

VK-RED commented Dec 14, 2024

Updated the routing

Screencast

Praashh Dec 18, 2024

Choose a reason for hiding this comment

VK-RED Dec 18, 2024

Choose a reason for hiding this comment

Praashh Dec 18, 2024

Choose a reason for hiding this comment

Praashh Dec 18, 2024

Choose a reason for hiding this comment

VK-RED Dec 18, 2024

Choose a reason for hiding this comment

Praashh Dec 18, 2024

Choose a reason for hiding this comment

Praashh Dec 18, 2024

Choose a reason for hiding this comment

VK-RED commented Dec 19, 2024