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 Strapi product list redirect fields #2165

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

masonmcelvain
Copy link
Contributor

@masonmcelvain masonmcelvain commented Dec 20, 2023

Connects #2067

The intent is to grab redirectTo at request time (cached with Redis) and issue a redirect if appropriate.

redirectFrom isn't necessary for the Next.js app, but offers a convenient way for Strapi admin users to create redirects from a source to a target from the source product list page. (e.g. If I am on product list A and want to cause product list B to redirect to product list A, I can do so without visiting product list B in Strapi admin).

https://docs.strapi.io/dev-docs/backend-customization/models#relations

qa_req 0

Copy link

vercel bot commented Dec 20, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-commerce ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 20, 2023 11:36pm
react-commerce-prod ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 20, 2023 11:36pm

@ghost
Copy link

ghost commented Dec 20, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

Copy link
Contributor

@jarstelfox jarstelfox left a comment

Choose a reason for hiding this comment

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

CR 🌵

Copy link
Contributor

github-actions bot commented Dec 20, 2023

📦 Next.js Bundle Analysis for @ifixit/commerce-frontend

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@danielbeardsley
Copy link
Member

e.g. If I am on product list A and want to cause product list B to redirect to product list A, I can do so without visiting product list B in Strapi admin

I feel like this is complexity that isn't really needed. We have to add another query to the graphQL query and it allows there to be two places where the same data is stored and thus there can be conflicts:

X.redirectFrom = Y 
Y.redirectTo = Z

Where do we go when visiting Y?

@masonmcelvain
Copy link
Contributor Author

I feel like this is complexity that isn't really needed

You're right, it's not necessary, but it could be nice to see which product lists redirect to the one you're looking at. It is also a visual cue to help Strapi admin users prevent redirect chains.

there can be conflicts

I don't think Strapi allows there to be conflicts. When you add a redirectFrom via the UI, the redirectTo is automatically set on the other product list, even if redirectTo is already set to something. Try it yourself: https://main.govinor.com/admin/content-manager/collectionType/api::product-list.product-list?page=1&pageSize=100&sort=title:ASC&plugins[i18n][locale]=en

We do not have to query for redirectFrom in Next.js, and I do not in the implementation here #2166.

@danielbeardsley
Copy link
Member

I don't think Strapi allows there to be conflicts.

Oh! I didn't understand the "inversedBy": "redirectFrom" before. I assumed this was two separate relationships, but instead it's more like giving a name to the other side of the relationship for easier reference / visibility. Nice!

Thanks for explaining!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants