volto icon indicating copy to clipboard operation
volto copied to clipboard

PLIP: Link Integrity View

Open tisto opened this issue 2 years ago • 24 comments

PLIP (Plone Improvement Proposal)

Responsible Persons

Proposer: Timo Stollenwerk

Seconder:

Abstract

Plone (Volto) should provide a dedicated view to show all items that are connected with a particular content object.

Motivation

Volto provides a link integrity view that shows relations and links that would break when deleting the object:

Inhalte (10)

Though, this overlay only shows up when the editor tries to delete an item. Editors usually hesitate to press the delete-button, just to check.

Assumptions

Proposal & Implementation

Implement a dedicated view "related items" that is added to the "..." menu in Volto that shows a list of all content objects that reference the current object.

Deliverables

Risks

Participants

Relation Types

  • related items (we don't use it in Volto)
  • link/referencing (deserializer with adaptors, stored in the relations catalog)
    • RichText / Slate / DraftJS
    • Slate Table
    • URL / href -> works for nested blocks
  • custom relation field

UX Questions / Tasks

  • pop up on delete
    • how do we show many relations?
    • do we group them by relation type
  • how can editors see up-front where an object is used?
    • tab in edit view
    • actions item in the left sidebar

tisto avatar Jan 30 '23 23:01 tisto

@ksuess @pbauer this is an idea we came up with (relations view per content object). Maybe we can build this on top of the work that you folks plan to do at the Alpine City Sprint. Looking forward to seeing you there!

tisto avatar Jan 30 '23 23:01 tisto

Superbe idea!

ksuess avatar Jan 31 '23 11:01 ksuess

The related items are not even shown at the end of the contents view, as in Plone Classic:

https://github.com/plone/volto/issues/3740

wesleybl avatar Feb 07 '23 19:02 wesleybl

Notes from Beethoven Sprint 2023 (Albert):

  • In general, the goal is to (during save) detect everywhere that a content item links to another content item, and update the list of isReferencing relations
  • Already handled:
    • Links in Dexterity richtext fields
    • Relation fields
    • Links in slate text block (and old draftjs text block)
    • Any url or href field in any block
    • Blocks nested inside columns, hrefList, or slides of another block (https://github.com/plone/plone.volto/blob/main/src/plone/volto/linkintegrity.py)
  • Not handled:
    • Links in slateTable block and custom blocks that use slate
    • Blocks nested inside “blocks”
    • Blocks which store a url in a field with a different name (can be added with a custom IBlockFieldLinkIntegrityRetriever adapter)
  • Challenges:
    • It’s difficult to automatically find links because the backend doesn’t know anything about the block schema, so has to recognize URLs from the data structure only
    • It’s annoying that we have to implement similar adapters to find links for 2 different things (resolveuid transform and linkintegrity)
  • Possible solutions:
    • Can we make linkintegrity use information collected by the resolveuid transform, so that only one set of adapters is needed?
    • Is there a better way to automatically recognize slate values and fields with a url? (https://github.com/plone/plone.restapi/pull/1595 could help, especially combined with the previous idea)
    • Send some sort of hints about the block schema to the backend? (e.g. https://github.com/plone/volto/issues/2252 )
  • Performance: Currently it always deletes the existing relations and creates new ones. There might be some performance improvement by only writing the actual changes. (https://github.com/plone/plone.app.linkintegrity/blob/master/plone/app/linkintegrity/handlers.py#L124)
  • UI: There is a PLIP about adding a new view for editors to check where an item has incoming links from: https://github.com/plone/volto/issues/4339 (currently this is only visible when trying to delete an item)
    • Do we have a mockup for that screen?

Discussion

A4B82B31-4081-4CE5-807A-B03D6E46AB0C_1_105_c

Mocks:

https://www.figma.com/file/0dMJLxvqhhwQ8DRxSGDbQj/Links?type=design&node-id=0%3A1&t=hV5Qf8D8h5OUxI6U-1

tisto avatar Aug 11 '23 02:08 tisto

@jaroel @jackahl @sneridagh @davisagli this is just an FYI of the current status of the "link integrity view" in Volto 17:

  • setting a link on the portal root (text block) does not show up
  • setting a link via a teaser shows up but in the "LINKING THIS ITEM WITH HYPERLINK IN TEXT" section, not in its own section
  • setting a link via a teaser within a grid does not show up
  • setting a link to an image via the image block shows up but in the "LINKING THIS ITEM WITH HYPERLINK IN TEXT" not in its own section
  • the "link integrity view" show an icon in the left toolbar with a link to the relations control panel (I am clueless why this is placed there and a user loses the entire context when clicking on the global relations control panel)

For the keynote tomorrow, I will just show the view. This is not ready to be shipped in a major release IMHO.

tisto avatar Oct 03 '23 16:10 tisto

@tisto fyi I've updated the preview screenshot at https://github.com/plone/volto/pull/5234

jaroel avatar Oct 03 '23 16:10 jaroel

@tisto

setting a link on the portal root (text block) does not show up

Needs investigation; just a guess: maybe there are some adapters needed for linkintegrity which are registered for IContentish, which the site root does not provide.

setting a link via a teaser shows up but in the "LINKING THIS ITEM WITH HYPERLINK IN TEXT" section, not in its own section setting a link to an image via the image block shows up but in the "LINKING THIS ITEM WITH HYPERLINK IN TEXT" not in its own section

tl;dr: this one is tricky.

The sections are based on the types of relationships stored in the relation catalog. All links found by plone.app.linkintegrity are stored with the "isReferencing" type. Custom RelationChoice fields can use other relation types. So, we aren't currently tracking the information needed to group links by what type of block they came from. "LINKING THIS ITEM WITH HYPERLINK IN TEXT" is simply an incomplete description of what is included in the isReferencing relation type. It's more like "links from blocks", except that's of course not a user-friendly way to explain it.

We probably need more discussion about what UX is needed here. If we need to group the links by block type then it will require deeper changes to plone.app.linkintegrity.

setting a link via a teaser within a grid does not show up

This is supposed to already be working. What versions of plone.restapi and plone.volto do you have?

the "link integrity view" show an icon in the left toolbar with a link to the relations control panel (I am clueless why this is placed there and a user loses the entire context when clicking on the global relations control panel)

I'm not sure what this is for either. @ksuess I think you added it, can you explain the use case?

davisagli avatar Oct 03 '23 16:10 davisagli

  • setting a link via a teaser within a grid does not show up

    • There's a test for this specifically: https://github.com/plone/plone.volto/blob/26e5fc1ed495aea740a5687029c7f860f2132b87/src/plone/volto/tests/test_linkintegrity.py#L29-L46
    • I noticed that initially the page didn't show the grid /w teaser, even when I had saved the page twice. A hard refresh resolved that, iirc.
  • the "link integrity view" show an icon in the left toolbar with a link to the relations control panel (I am clueless why this is placed there and a user loses the entire context when clicking on the global relations control panel)

    • That's because we try to mimic ++skin++zmi traversal-like behavior. See for example https://github.com/plone/volto/pull/5292 and https://github.com/plone/volto/blob/8caa10f07fbc2fcc97a3bfb95af80e4c39cb763f/src/components/manage/LinksToItem/LinksToItem.jsx#L49-L52
    • For this specific view I'll happily remove the link - I don't see a path from a control panel.

jaroel avatar Oct 10 '23 09:10 jaroel

  • setting a link on the portal root (text block) does not show up
    • Removing IContentish does not have an effect. Adding IContentish by removing the classImplementsOnly(PloneSite, implementedBy(PloneSite) - IContentish) line makes Products.CMFCore.explicitacquisition raise a NotFound though :/
    • plone.retapi.interfaces.IBlocks is correctly set for PloneSite.

Any idea where does the current (working for Page objects) relation related logic reside?

jaroel avatar Oct 10 '23 15:10 jaroel

@jaroel

  1. plone.app.linkintegrity handler for ObjectModifiedEvent which updates relations using IRetriever adapters to find the current links: https://github.com/plone/plone.app.linkintegrity/blob/master/plone/app/linkintegrity/handlers.py#L106
  2. IRetriever adapter for blocks: https://github.com/plone/plone.restapi/blob/main/src/plone/restapi/blocks_linkintegrity.py

davisagli avatar Oct 10 '23 16:10 davisagli

The event handlers aren't called for PloneSite. They are when I add <include package="plone.app.relationfield" /> <class class="Products.CMFPlone.Portal.PloneSite"> <implements interface="plone.app.relationfield.interfaces.IDexterityHasRelations" /> </class> to plone.app.linkintegrity:configure.zcml.

Looks like this when it works: Screenshot 2023-10-11 at 10 45 39

The interface should already exist on PloneSite.

jaroel avatar Oct 11 '23 08:10 jaroel

When I unremove IContentish things are wired up correctly again. IDexterityHasRelations is applied to the PloneSite without the above class zcml thingy.

jaroel avatar Oct 11 '23 10:10 jaroel

https://github.com/plone/Products.CMFPlone/issues/3833 is relevant.

jaroel avatar Oct 11 '23 12:10 jaroel

the "link integrity view" show an icon in the left toolbar with a link to the relations control panel (I am clueless why this is placed there and a user loses the entire context when clicking on the global relations control panel)

I'm not sure what this is for either. @ksuess I think you added it, can you explain the use case?

My focus is on custom relations. Therfore a link to the panel where relations can be managed/created/modified/deleted is usefull. But if you think the link to the panel is distracting, then you may want to wrap the toolbar button with an opt-in flag condition of config.settings.

ksuess avatar Oct 17 '23 11:10 ksuess

@jaroel @sneridagh @davisagli I am writing the Plone roadmap for Plone 6.x and beyond and I am not able to find the PR that introduces the new "link integrity" view that is reachable when clicking on the "..." in the Volto menu.

Grepping for "link integrity" in the changelog:

https://github.com/plone/volto/blob/main/packages/volto/CHANGELOG.md

or looking for closed PRs did not do the trick:

https://github.com/plone/volto/pulls?q=is%3Apr+is%3Aclosed+link+integrity

Am I missing something here?

tisto avatar Feb 12 '24 16:02 tisto

@tisto It's here: https://github.com/plone/volto/pull/4787 -- in the UI the view is called "Links and References", not "link integrity"

davisagli avatar Feb 12 '24 16:02 davisagli

Thanks @davisagli!

tisto avatar Feb 12 '24 16:02 tisto

@stevepiercy I am working on the Plone 6.x roadmap and I would include links to the documentation of a feature. @pgrunewald added proper docs to Volto in his PR:

https://github.com/plone/volto/pull/4787/files#diff-dac7982e86cb5774129a24e89f27bcb5e3e088a270c1b879629fb5d1dba5d1c1

I would like to include a link to the official Plone 6.1 docs if possible. However, I guess we do not generate them yet, correct?

@pgrunewald do I remember correctly that you worked on this during Beethoven Sprint last year together with @danalvrz? I'd like to give people proper credit in our roadmap document.

tisto avatar Feb 12 '24 16:02 tisto

@stevepiercy ok, I found the link to the docs:

https://6.docs.plone.org/volto/user-manual/links-to-item.html

Though, I realized a problem here;

In the Volto 17 readme, we link to the Plone 6 docs:

https://www.npmjs.com/package/@plone/volto/v/17.2.0

The new feature from Volto 17 and Plone 6.1 is included. However, people who read the docs might expect this feature to be present in Plone 6.

I understand that it is lots of work to keep docs for the major Plone versions in shape. I just wanted to point this out when I realized it.

tisto avatar Feb 12 '24 16:02 tisto

@sneridagh @plone/volto-team I guess we can close this PLIP as completed, do you agree?

tisto avatar Feb 12 '24 19:02 tisto

@tisto https://6.docs.plone.org/ is the official site for all Plone 6 docs. We are so far behind converting 5.2 docs to 6, and too few people working on docs, that forging ahead with a https://61.docs.plone.org/ would be an unfunded and unsupported activity.

stevepiercy avatar Feb 13 '24 00:02 stevepiercy

@stevepiercy @tisto I started a PR to mention what version the feature was added, but I have some questions about how to cover it properly: https://github.com/plone/volto/pull/5756

davisagli avatar Feb 13 '24 05:02 davisagli

@pgrunewald do I remember correctly that you worked on this during Beethoven Sprint last year together with @danalvrz? I'd like to give people proper credit in our roadmap document.

@tisto The "links and references" view, former called "link integrity" view, is a work of @pgrunewald and me. Paul started with a view for links at the sprint, but did not continue working afterwards. While working on the relations control panel I finished the view and enhanced it by showing all relations, not only links. Worth mentioning is, that @jackahl was so kind to write tests recently to complete the picture.

ksuess avatar Feb 15 '24 10:02 ksuess

I can concur with @ksuess. While @danalvrz worked on the link integrity error message, I worked during the sprint on the link integrity view, which was greatly improved by Katja later on.

pgrunewald avatar Feb 15 '24 11:02 pgrunewald

This was completed for Plone 6.1: https://6.docs.plone.org/volto/user-manual/links-to-item.html

davisagli avatar Aug 05 '24 18:08 davisagli