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

Broken page / Broken link reports do not work with elemental content area #689

Open
3 tasks
maxime-rainville opened this issue Jul 8, 2019 · 12 comments
Open
3 tasks

Comments

@maxime-rainville
Copy link
Contributor

maxime-rainville commented Jul 8, 2019

Description

The "broken link" and "page with broken file" report do not include pages with elemental area containing broken links.

Steps to reproduce

  • Create a new elemental page
  • Add content block.
  • Add a link to a new file in the content block
  • Add a link to a new page in the content block
  • Publish the elemental page
  • Archived the new page and delete the new file.
  • Go look at the reports

Expected behavior: The elemental page should appear in the broken link report and the page with broken files report

Actual behaviour: It doesn't.

Originally created at silverstripe/silverstripe-cms#2453

ACs

  • This looks through every block with a HTML field (including custom blocks)
  • Content is being surfaced for the two reports outlined in the replication steps
  • Create a new issue to create a build task to retrospectively find broken links within existing blocks (assuming that only future links will be picked up after this fix)
  • Investigate and ensure this will not result in a regression to the dependant pages feature (see note)

Notes

Related

PRs

@ScopeyNZ
Copy link
Contributor

ScopeyNZ commented Jul 9, 2019

Haha I know @chillu just told you to move this, but any fix here is going to be an annoying kludge. Content is not just on the $Content field which CMS often assumes. I'll attempt to start some discussion on this in the CMS repo later today.

@maxime-rainville
Copy link
Contributor Author

My guess is whatever fix we come up with will probably have an impact on both CMS and Elemental.

I guess it depends on whether we choose to look at this as an Elemental problem first or a CMS problem first.

@michalkleiner
Copy link
Contributor

My 2c would be to not save a new version of each page where a broken link is found when the report is run, if possible.

@ScopeyNZ
Copy link
Contributor

ScopeyNZ commented Jul 9, 2019

My 2c would be to not save a new version of each page where a broken link is found when the report is run, if possible.

This just means using ->writeWithoutVersion when updating the content (as it rewrites the anchor to show invalid links). I wonder if this should be configurable UX experience? I guess it's weighing up whether a content author cares more about what caused their links to go red in their editor or why there's an extra version listed on the page. (cc @silverstripeux)

@michalkleiner
Copy link
Contributor

One of our client's editor's behaviour was "ah, something/someone created modified versions of some pages (yellow dot in the site tree), let's bulk publish them just to be sure" — effect of running broken links report. Then they had bulk published couple pages every week and half a year later it was impossible to determine which versions/updates where legit and which were only a consequence of a weird UX of the report.
I would totally vote for an option to be able to turn "highlight broken links in red" feature. Just keep the list of them in the report.

@robbieaverill
Copy link
Contributor

We've got an issue to track the missing feature required to fix this here: silverstripe/silverstripe-cms#2454

@brynwhyman
Copy link

This issue was parked for the discussion on RFC: Introduce a consistent content API to begin. However, we haven't been able to get any traction, nor dedicate time to that RFC likely given it's broad scope.

If this bug isn't impact/critical, it's very close as the reports arerendered useless if content blocks are the main source of content for a site. I'm starting to think an Elemental-specific fix could be the initial way forward here?

@clarkepaul
Copy link

Or remove the report if blocks are installed perhaps.

@emteknetnz
Copy link
Member

emteknetnz commented Jul 30, 2020

Seems like that RFC would need to be a Silverstripe 5 thing, I cannot see anyway we could get something like that into 4 without either:
a) breaking everything, or
b) ending up with two ways to do something, which to me is worse then just having one

In any case it's a lot of work, agree with @brynwhyman that we should do an Elemental-specific fix here

@brynwhyman
Copy link

Or remove the report if blocks are installed perhaps.

These reports are part of the core recipe (with the external broken links report also provided to CWP sites). I feel like if we can get elemental to workaround this issue and keep the functionality, it seems like something worth doing for now. There will be a lot of sites with both these reports and elemental installed

@emteknetnz
Copy link
Member

Fixed in #822

Note: only fixes internal links / files, not external links which is done in the externallinks module

@brynwhyman
Copy link

I've raised a separate issue to track the external-links module concern, see: silverstripe/silverstripe-externallinks#68

I've updated the ACs to remove this report as a concern for this issue.

@emteknetnz emteknetnz assigned emteknetnz and unassigned emteknetnz Sep 14, 2020
@Cheddam Cheddam self-assigned this Nov 17, 2020
@Cheddam Cheddam added this to the Sprint 72 milestone Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants