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: redirects.yaml #11085

Closed
ErisDS opened this issue Sep 6, 2019 · 12 comments · Fixed by #12187
Closed

Feature: redirects.yaml #11085

ErisDS opened this issue Sep 6, 2019 · 12 comments · Fixed by #12187
Labels
feature [triage] New features we're planning or working on help wanted [triage] Ideal issues for contributors to help with pinned [triage] Ignored by stalebot

Comments

@ErisDS
Copy link
Member

ErisDS commented Sep 6, 2019

Our redirect feature is cool and powerful, but having to correctly format a JSON file by hand is HORRIBLE.

We are now using YAML for user-edited settings files, we should do that for redirects as well.

Ref material

Our current JSON redirects format looks like:

{
  "from": "/url/" ,
  "to":  "/url/",
  "permanent": true | false
}
  • We support full JavaScript regexes in the from field
  • Regex patterns e.g. $1, $2 so on can be substituted into the to field.
  • Redirects can be either permanent (301) redirects, or temporary (302) redirects, based on the permanent flag.
  • Also (currently undocumented) we support adding an i flag to the end of the from path, e.g. "from": "/my-url/i" which makes it do a case insensitive match.

Statamic have a similar feature, provided in YAML: https://docs.statamic.com/routing#redirects

Netlify use a tab separated values style: https://www.netlify.com/docs/redirects/

Spec

We will add support for uploading a redirects file called redirects.yaml. The file must contain valid YAML.

Redirects will apply to the web application of Ghost only (same as now).

This new file will support all the same features as the old redirects.json format but in an easier to write and maintain style. The main change is that instead of a property, the type of redirect is specified via one of two top-level keys.

Full example:

302:
  /from-url/: /to-url/

301:
  /category/([a-z0-9\-]+)/i: /tag/$1/
  /v([0-9\.]+)/docs/([a-z0-9\-]+)/i: /docs/$2/

Migration

For now we should just support both formats. In the next major version we can drop support for uploading the JSON format.

We may be able to write a migration for a major upgrade using something like https://www.npmjs.com/package/json2yaml

However JSON is valid YAML 🤔

@ErisDS ErisDS added help wanted [triage] Ideal issues for contributors to help with feature [triage] New features we're planning or working on labels Sep 6, 2019
@Assisting
Copy link

I've been using Ghost for over a year, it's probably time to give something back; I'll take this on!

Assisting added a commit to Assisting/Ghost that referenced this issue Oct 9, 2019
…as a fallback

refs TryGhost#11085
No migration tool, that will still take some doing.
Assisting added a commit to Assisting/Ghost that referenced this issue Oct 9, 2019
@stale
Copy link

stale bot commented Jan 3, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale [triage] Issues that were closed to to lack of traction label Jan 3, 2020
@naz naz removed the stale [triage] Issues that were closed to to lack of traction label Jan 9, 2020
@naz
Copy link
Contributor

naz commented Jan 9, 2020

This feature has almost been finished with #11218, easy candidate for somebody from the community to pick up 😉

@Assisting
Copy link

Yeah, that's my bad; winter got weird IRL. If nobody else picks it up I'll see where I can get this weekend.

@Assisting
Copy link

I'm getting a bit caught up on the unit testing framework, although the configuration work went OK.

@naz
Copy link
Contributor

naz commented Jan 27, 2020

@Assisting let me know if I could help with any guidance and feel free to ask any related questions 😉

@sainthkh sainthkh mentioned this issue Sep 8, 2020
8 tasks
sainthkh added a commit to sainthkh/Ghost-Admin that referenced this issue Sep 10, 2020
@naz
Copy link
Contributor

naz commented Nov 2, 2020

@sainthkh to be clear, think it would be best if we wrap up current PR with the scope it already touched. We can add the aliasing proposed here and more improvements in future iterations. Wanted to avoid bloating with features as the branch has been running for long enough already :)

@sainthkh
Copy link
Contributor

sainthkh commented Nov 2, 2020

Right. I agree with you. It seems that we can organize leftover todos in this way.

Current PRs

  • Add endpoints and change them in admin client.
  • copy review and apply them.
  • check/change setFromFilePath signature

Future PRs

  • aliased routes => I think it's a good candidate for good first issue.

Maybe

  • 307 redirect.

sainthkh added a commit to sainthkh/Ghost that referenced this issue Nov 3, 2020
sainthkh added a commit to sainthkh/Ghost that referenced this issue Nov 3, 2020
issue TryGhost#11085

startGhost test util function copies yaml file when a redirectsFileExt option is given.
sainthkh added a commit to sainthkh/Ghost that referenced this issue Nov 3, 2020
issue TryGhost#11085

The parsing code inside custom-redirects.js has been moved to frontend/services/redirects/seetings.js. I found that it was already imported in the file.
sainthkh added a commit to sainthkh/Ghost that referenced this issue Nov 3, 2020
sainthkh added a commit to sainthkh/Ghost that referenced this issue Nov 3, 2020
sainthkh added a commit to sainthkh/Ghost that referenced this issue Nov 3, 2020
sainthkh added a commit to sainthkh/Ghost that referenced this issue Nov 3, 2020
sainthkh added a commit to sainthkh/Ghost that referenced this issue Nov 3, 2020
sainthkh added a commit to sainthkh/Ghost that referenced this issue Nov 3, 2020
sainthkh added a commit to sainthkh/Ghost that referenced this issue Nov 3, 2020
sainthkh added a commit to sainthkh/Ghost-Admin that referenced this issue Nov 3, 2020
@sainthkh
Copy link
Contributor

sainthkh commented Nov 3, 2020

@naz all things clear. Waiting for review and merge.

@naz naz closed this as completed in #12187 Nov 3, 2020
naz pushed a commit that referenced this issue Nov 3, 2020
closes #11085

- Ghost has been using YAML format for other configurations (e.g. routes). The plan is to move to this format for all user-edited settings files. By default JSON format is still used in Ghost Admin API v2/v3, but will be changed to YAML in API v4. Check referenced issue for more context.
- New format supports all the features available before. The main noticeable change is the structure of config file. It is now grouped by redirect HTTP code instead of specifying `"permanent": true | false` attribute for each config property. Example format for YAML config:
```
302:
  /from-url/: /to-url/

301:
  /category/([a-z0-9\-]+)/i: /tag/$1/
  /v([0-9\.]+)/docs/([a-z0-9\-]+)/i: /docs/$2/
```
- Added 2 new endpoints: `POST redirects/upload` and `GET redirects/download`. These serve as an alias to current GET/POST `/redirects/json. "upload/download" naming pattern is introduced to match the convention with other resources that can be uploaded and downloaded (images, themes etc.). `/redirects/json`  endpoints will be removed in Admin API v4
- The parsing code from `custom-redirects.js` has been moved to `frontend/services/redirects/settings.js`. This location is more appropriate for this logic and eventually `custom-redirects.js` middlewear might be moved into "frontend" as this middlewear plays a role mostly effecting that area.
naz added a commit that referenced this issue Nov 3, 2020
refs #11085

- The logic which uses this error deals with an input yaml file thus the change.
- Related discussion here - #12187 (comment)
naz added a commit that referenced this issue Nov 4, 2020
refs #11085

- Incorrect usage error was logged to the output when there was no recirecst configuration file present in the system. Previously an empty string was returned in such situation, resulting in "ENOENT" error, which was ignored through special handling.
- The fix resembles logic in redirects async getter function where empty array is returned when the config file does not exits.
- Attempting to read unexistent config should not ever happen and will be handled on the config service layer, this is why special "ENOENT" handling has been removed
naz added a commit that referenced this issue Nov 4, 2020
naz pushed a commit to sainthkh/Ghost-Admin that referenced this issue Feb 18, 2021
naz pushed a commit to TryGhost/Admin that referenced this issue Feb 22, 2021
refs TryGhost/Ghost#11085
refs TryGhost/Ghost#12187

- YAML file support has been added to Ghost's API through referenced above PR. YAML will become a default supported file in the next version of Ghost replacing JSON redirects
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature [triage] New features we're planning or working on help wanted [triage] Ideal issues for contributors to help with pinned [triage] Ignored by stalebot
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants