silverstripe / silverstripe-elemental

Create pages in Silverstripe CMS using content blocks
http://dna.co.nz
BSD 3-Clause "New" or "Revised" License
110 stars 115 forks source link

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

Open maxime-rainville opened 5 years ago

maxime-rainville commented 5 years ago

Description

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

Steps to reproduce

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 https://github.com/silverstripe/silverstripe-cms/issues/2453

ACs

Notes

Related

PRs

ScopeyNZ commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

We've got an issue to track the missing feature required to fix this here: https://github.com/silverstripe/silverstripe-cms/issues/2454

brynwhyman commented 4 years ago

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 commented 4 years ago

Or remove the report if blocks are installed perhaps.

emteknetnz commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

Fixed in https://github.com/dnadesign/silverstripe-elemental/pull/822

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

brynwhyman commented 4 years ago

I've raised a separate issue to track the external-links module concern, see: https://github.com/silverstripe/silverstripe-externallinks/issues/68

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