feat: update metadata and redirects #435
Conversation
|
This PR will trigger a minor release when merged. |
There was a problem hiding this comment.
Looks good so far in terms of functionality 👍🏻
Since this fix is very specific (updating and publishing redirect with a first audit type that requires it), I'd propose either to:
- make it specific to broken backlinks, i.e. check the audit type and run only for it (return a not supported otherwise), and document accordingly, until we have another kind of auto-fix to implement which will inform a better and/or generic API
- or try to come up with a more generic API/handling in the controller, by looking at known auto-fixes (redirects), but also potential next ones (bulk-metadata).
src/controllers/audits.js
Outdated
| return notFound('Audit type not found'); | ||
| } | ||
| const existingFixedURLs = config.getFixedURLs(auditType); | ||
| const newFixedURLs = mergeFixes(existingFixedURLs, fixedURLs); |
There was a problem hiding this comment.
What's the reasoning behind merging fixed URLs?
src/controllers/audits.js
Outdated
| const existingFixedURLs = config.getFixedURLs(auditType); | ||
| const newFixedURLs = mergeFixes(existingFixedURLs, fixedURLs); | ||
| const contentClient = await getContentClient(env, site, log); | ||
| for (const { brokenTargetURL, targetURL } of fixedURLs) { |
There was a problem hiding this comment.
Before appending the fixed URLs to the redirects sheet, we need to check if they don't already exist in the file.
This is the minimal validation. Other kinds of validation (can follow-up) are listed in https://wiki.corp.adobe.com/display/AEMSites/SEO+%7C+Broken+Backlinks+Manual+and+Auto-Fix#SEO|BrokenBacklinksManualandAutoFix-Validation
src/support/utils.js
Outdated
|
|
||
| /* c8 ignore next 3 */ | ||
| export const { fetch } = process.env.HELIX_FETCH_FORCE_HTTP1 | ||
| ? h1() | ||
| : h2(); | ||
|
|
||
| export const GOOGLE_DRIVE = 'google-drive'; |
There was a problem hiding this comment.
Maybe all these constants and methods related to the content client should go in an utils of their own?
alinarublea
changed the title
Please ensure your pull request adheres to the following guidelines:
describe here the problem you're solving.
If the PR is changing the API specification:
yet. Ideally, return a 501 status code with a message explaining the feature is not implemented yet.
If the PR is changing the API implementation or an entity exposed through the API:
Related Issues
Thanks for contributing!