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

PLIP: Link Integrity View #4339

Closed
tisto opened this issue Jan 30, 2023 · 25 comments
Closed

PLIP: Link Integrity View #4339

tisto opened this issue Jan 30, 2023 · 25 comments

Comments

@tisto
Copy link
Member

tisto commented Jan 30, 2023

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
Copy link
Member Author

tisto commented Jan 30, 2023

@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!

@ksuess
Copy link
Member

ksuess commented Jan 31, 2023

Superbe idea!

@wesleybl
Copy link
Member

wesleybl commented Feb 7, 2023

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

#3740

@tisto
Copy link
Member Author

tisto commented Aug 11, 2023

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:
  • 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:
  • 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: PLIP: Link Integrity View #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
Copy link
Member Author

tisto commented Oct 3, 2023

@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.

@jaroel
Copy link
Member

jaroel commented Oct 3, 2023

@tisto fyi I've updated the preview screenshot at #5234

@davisagli
Copy link
Member

@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?

@tisto tisto changed the title Related Items / Relations / Link Integrity View PLIP: Link Integrity View Oct 7, 2023
@tisto tisto added this to the Plone 6.1 milestone Oct 7, 2023
@jaroel
Copy link
Member

jaroel commented Oct 10, 2023

@jaroel
Copy link
Member

jaroel commented Oct 10, 2023

  • 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?

@davisagli
Copy link
Member

@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

@jaroel
Copy link
Member

jaroel commented Oct 11, 2023

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
Copy link
Member

jaroel commented Oct 11, 2023

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

@jaroel
Copy link
Member

jaroel commented Oct 11, 2023

plone/Products.CMFPlone#3833 is relevant.

@ksuess
Copy link
Member

ksuess commented Oct 17, 2023

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.

@tisto
Copy link
Member Author

tisto commented Feb 12, 2024

@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?

@davisagli
Copy link
Member

@tisto It's here: #4787 -- in the UI the view is called "Links and References", not "link integrity"

@tisto
Copy link
Member Author

tisto commented Feb 12, 2024

Thanks @davisagli!

@tisto
Copy link
Member Author

tisto commented Feb 12, 2024

@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
Copy link
Member Author

tisto commented Feb 12, 2024

@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
Copy link
Member Author

tisto commented Feb 12, 2024

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

@stevepiercy
Copy link
Collaborator

@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.

@davisagli
Copy link
Member

@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: #5756

@plone plone locked and limited conversation to collaborators Feb 14, 2024
@ksuess
Copy link
Member

ksuess commented Feb 15, 2024

@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.

@pgrunewald
Copy link
Contributor

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.

@davisagli
Copy link
Member

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

@github-project-automation github-project-automation bot moved this from Backlog to Done in Volto Roadmap Aug 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Status: Done
Status: Done
Development

No branches or pull requests

7 participants