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

[feature request] How-to redirect url when name changes #1168

Closed
darigovresearch opened this issue Jun 6, 2021 · 25 comments
Closed

[feature request] How-to redirect url when name changes #1168

darigovresearch opened this issue Jun 6, 2021 · 25 comments

Comments

@darigovresearch
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Often if you change the name of the How to it will update the URL but old links to the previous URL no longer works.

Describe the solution you'd like
It would be great if we can keep the old link active but it redirects to the new URL that way any links to the old URL will still be valid.

Describe alternatives you've considered
It doesn't need to be automatic, can be a static site saying that the page has been updated.

Additional context
GitHub does this when you change the name of the repository so there may be an easy way to implement this.

@chrismclarke
Copy link
Member

Make a lot of sense to me, I think the way wordpress handles it is keeping a list of previous URLs redirects when a post changes, which it uses when a page can't be found directly. We could probably try to do something similar either:

A) Inside a post itself, have a previousUrls (or similar) property that can store an array of all the slugs a howto has had in the past, or

B) Store a single list of redirects (or similar) where we keep them all.

I think probably the simplest to me would be option A, and adding a function when the user saves the howto to populate the list of previous URLs. We would then want to add a method to the howto store when looking up a howto, so that if the URL is not found we can run an array-contains query across all howtos to find any which have the previousURL saved. If there are multiple found then probably just return the first (maybe show a list of all in the future)

@chrismclarke
Copy link
Member

One other option which would be much faster in the short term, would just be to redirect to the howto page and with the unmatched url in the search bar. It won't be perfect but would still likely have the target howto somewhere near the top assuming a few keywords are matched,

E.g. if someone tries to visit: https://community.preciousplastic.com/how-to/create-a-lamp (does not exist), they would see:

image

@chrismclarke
Copy link
Member

Have tagged with help wanted in case anyone fancies taking this one on?

@thisislawatts
Copy link
Collaborator

@chrismclarke Happy to take a look into this :)

I really like your suggestion of redirecting to search if there are no exact matches found. I think that is a helpful default behaviour.

In terms of option A or B from your earlier comment I think there should be a limit to the number of previous URLs stored. The path /how-to is a global namespace so we want to avoid it becoming too littered with old redirects preventing new how-tos replacing old ones.

Having not worked with the codebase before do you have any opinion on A vs B?
A seems simpler but not knowing much about the system scale/behaviour I don't know how expensive the URL lookup within an array column would be. 💭

@chrismclarke
Copy link
Member

Sounds awesome, great if you want to take this one on.

I think the main thing to avoid like you say is things becoming too littered (or case where we keep redirecting from one place to another and possibly into infinity (and beyond)).

I think this is easiest avoided if we go for option A, so that we can run a single query to find any howtos previously with that slug and not jump from link to link. From a practical side I think this means when we try look up a howto URL the general flow should be something like:

  1. Query for exact match, load if found (existing behaviour)
  2. Query across all howtos for any that contain previous matching URL (redirect to first match if found)
  3. Redirect to howto list page with search filled and maybe a message to say "The link appears to be broken, did you mean one of these?" (or something along those lines)

Then when it comes to editing a howto probably we want to have some sort of message or warning if the title is changed to make sure the user understands links will break; but can discuss with Dave how best that could work.

Overall that makes the scope of this issue pretty large, so I'd probably recommend starting first just with the redirect to search page as a smaller issue (step 3 in the process), and then come back to fill in step 2 as a follow-up issue. Might also give a nicer introduction to the backend part of the database

@thisislawatts
Copy link
Collaborator

Sounds good to me @chrismclarke, incremental delivery all the way. :shipit:

So to clarify there are two tasks to action here:

  1. A 302 redirect to the search state instead of returning 404 for only howto entries, estimated size small
  2. Extend data model for howtos entitie to store a limited history of renaming and provide redirects, estimated size bigger than 1. 😉

Happy to work on both of these although I would start with 1. and see it merged/released before picking up 2.

So happy for someone to pick up 2 if they like. Otherwise please feel free to assign it to me if that's an acceptable practise here.

@chrismclarke
Copy link
Member

Sounds accurate to me!

For (1) - yes only howtos - that's currently the only place we've implemented a search feature so all that makes sense for now. I see the PR you've just opened and that looks ideal!

For (2) - yes, probably best to store as an array to keep the flexibility if we ever do want to store more than one previous URL (and can use the frontend to try and discourage perhaps).

I think the main catch/gotcha here might be how to implement the query - Firestore has a nice method to query arrays (e.g. find howto where 'some-slug' in array 'previousSlugs' (or whatever we decide): https://firebase.google.com/docs/firestore/query-data/queries#in_not-in_and_array-contains-any; however likely the method won't be exposed within the existing system of markup (basically because we have 3 different databases used in the platform, by default most commands are set only to recognise things common to all - so might take a bit of fiddling around whilst asserting this will be a server-only query)

And definitely happy if you want to keep working on this, I'll create a new issue to link to the open PR (404 redirect) and we can keep this one open for continued work.

@thisislawatts
Copy link
Collaborator

So I wrote up a long form idea which I will include below, but I wonder if the simplest possible solution to this problem is to not change a page's URL after it has been created. wdyt? @chrismclarke


Handle redirects for renamed content

Context:
User's can rename howtos as often as they like, which also affects the slug
of the page.

When the slug changes it means any links which exist to this content elsewhere across the web
will be broken.

Instead we support a history of changes to the permalink so that we can redirect users to the latest
version of the document.

Considerations:

  • The /how-to/* namespace is global, so we want to ensure it can not be polluted by a user repeatedly changing the article name and occupying the entire path.

For example:
1. A user creates a new howto (HowtoA) with the slug of how-to/version-1
2. A user changes the howto title using how-to/version-2
3. A user changes the howto title using how-to/version-3
4. As a result of this above title churn, the user now has 3 URLs which would redirect to a single article.
We need to consider how much history we should keep for a single article.

  • New how-tos must not be created which would occupy the same URL.
    Based on the previous example:
  1. A second user creates a new how to with the proposed slug of how-to/example, this would not be possible
    as the URL is already tracked as part of the history for (HowtoA)

Proposal:

A new entity in our domain model called URLRedirect which would contain the following properties.

  • path: a string representing the original URL ie. /how-to/version-1
  • documentType: an "enum" whose value would be one of the available document types
  • documentId: Unique internal id for the target document.
  • created: Date

The URLRedirects collection would be queried only when:

  • An attempt to load /how-to/an-example failed and before returning a 404
  • When looking to validate a new slug

Further questions:

  • How much history should we store?

@chrismclarke
Copy link
Member

If I'm understanding right, we're making the tradeoff that would allow multiple same slugs to exist following renames, so that one howto can't block out names permanently? Seems like a reasonable trade-off to me, and also keen to move the system outside just the howtos to make it easier to implement across modules.

As I take it, this could lead to a few different cases. For example, if we have 3 howtos with the following naming history (where all the legacy names are stored in a redirects collection with additional info as outlined)

howto_1 : [my_howto, my_awesome_howto]
howto_2: [howto_the_second ]
my_awesome_howto: [howto_3, my_howto]

And the following search cases:
howto_1 - returns correct howto immediately
howto_the_second - redirects correctly to howto_2
my_howto - query returns 2 results so possibly present both to choose from
my_awesome_howto - return the 3rd howto, so legacy links to howto_1 will be incorrectly directed

I think that covers all the cases? If so I think I'm ok with all of them (seems to be a fair trade-off). Although a couple suggestions I might have:

1 - When a user has authored a howto we also give them both a 'friendly' link (i.e. slug) and a permalink (e.g. /howto/?id=howto_unique_id), so that if they want to link from external sources they can. Ideally we will have better SEO in place by then so that when shared the permalink still populates a nice title/description to the link

2 - Possibly considering a purge system (maybe programmatic, maybe on demand), where legacy redirect names could be removed (not sure how best to implement of if actually required)

@davehakkens davehakkens added Bounty Level 3 Bounty Level #3 and removed bounty This is a paid job. labels Nov 1, 2022
@davehakkens
Copy link
Contributor

@Kiebert not sure if this falls in your expertise or intrest. But this would still be a useful one to take on! :)

@Kiebert
Copy link
Contributor

Kiebert commented Nov 2, 2022

Hi @davehakkens.
This week I am pretty occupied but next week I will reserve time for this issue.

@Kiebert
Copy link
Contributor

Kiebert commented Nov 24, 2022

Hi @davehakkens When working on the code I can't find a way to 'moderate' the created 'how-to' page.
Is there a trick to make the 'Awaiting moderation' sign dissappear?
image

@davehakkens
Copy link
Contributor

You deed to be admin, then you can approve them.
(but sometimes the 'Awaiting moderation' label sticks around due to caching)

@Kiebert
Copy link
Contributor

Kiebert commented Nov 25, 2022

Ok. And where is the 'Approve' button to be found?

@davehakkens
Copy link
Contributor

Should be on top. When you click on the how-to the ✓
afbeelding

@Kiebert
Copy link
Contributor

Kiebert commented Nov 26, 2022

Probably my account doesn't have the right rights to see these buttons.
I am able to access the admin section and see the 'how-to' that need approval.
image
This is in the following state of the local environment:
image
Is there an account login that has the right rights?

@davehakkens
Copy link
Contributor

davehakkens commented Nov 26, 2022 via email

@Kiebert
Copy link
Contributor

Kiebert commented Nov 26, 2022

Ah check. The 'Admin' option links to 'localhost/admin_v2'.
When I use the url: 'localhost/admin' I am getting a list of 'Super-admins' and 'Admins'.
There I was able to add my account as 'admin' and now I get the 'approval' option (after re-login).
Thanks for explanation.

@davehakkens
Copy link
Contributor

cool.Yes Its the old vs new admin panel.
In the old one you can assign admin roles to users.
It should also happen when you toggle here in the menubar.
But sounds like it doest assign fully?
afbeelding

@davehakkens
Copy link
Contributor

hey @Kiebert just checking in how this is going?

@Kiebert
Copy link
Contributor

Kiebert commented Dec 8, 2022

Hi @davehakkens pretty busy with other things like work.
I hope to have some time the upcoming Christmas holidays.
If that doesn't work out I will report it here.

@davehakkens
Copy link
Contributor

thanks for the update. No problem, take your time.
But yeh let us know if you wont manage :)

@Kiebert
Copy link
Contributor

Kiebert commented Jan 7, 2023

Hello.
As promised, here is the PR for this feature: #2064

Only this commit error... I don't know what I should do here, it looks like a dependency problem? https://app.circleci.com/pipelines/github/ONEARMY/community-platform/3578/workflows/d37b76e9-ce4f-4822-bea8-069cc04c67e3/jobs/22859

@davehakkens
Copy link
Contributor

thanks @Kiebert>
Indeed we have some issues with Lint atm.
Will look into that and review.

@chrismclarke
Copy link
Member

I think there's a fix for that included in #2065, so I'll try get that merged in to fix the linting issue.

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

Successfully merging a pull request may close this issue.

5 participants