plone
Link Integrity
Solves #2396
Requires plone.restapi (https://github.com/plone/plone.restapi/pull/953)
Deploy Preview for volto canceled.
| Name | Link |
|---|---|
| Latest commit | 1ec56615784206b0a8cae42dee67a3f526bd54a4 |
| Latest deploy log | https://app.netlify.com/sites/volto/deploys/6352708a8f7f6400082a6ff8 |
![netlify[bot] avatar](https://avatars.githubusercontent.com/in/13473?v=4 "Published by a pub.dev verified publisher"){kind=small}
Test summary
Run details
| Project | Volto |
| Status | Passed |
| Commit | 1ec5661578 |
| Started | Oct 21, 2022 10:16 AM |
| Ended | Oct 21, 2022 10:29 AM |
| Duration | 13:14 💡 |
| OS | Linux Ubuntu - 20.04 |
| Browser | Chrome 106  |
View run in Cypress Dashboard ➡️
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard
![cypress[bot] avatar](https://avatars.githubusercontent.com/in/9078?v=4 "Published by a pub.dev verified publisher"){kind=small}
It seems we already had a rotten PR going on: https://github.com/plone/volto/pull/2783 😫 that did the same thing.
@danielamormocea could you take a look and try to merge them (if there's something valuable, that I'm not aware now, i18n maybe?) there that we could reuse in this one? I'm biassed to use this one though, make sure that the i18n is also in sync.
As soon as we have the PR merged in p.restapi we can merge this one.
@danielamormocea @avoinea we changed the endpoint method from POST to GET, so i think that you have to change something in the frontend call
@danielamormocea I think they are changing the backend PR a bit. Please talk to @cekk to adjust
@sneridagh I checked their PR and adjusted the code here, so we have a GET req now. I also added de i18n.
@danielamormocea awesome work! I just added the plone.restapi checkout to this PR. I would love to see a Cypress test that covers the basic use cases:
- delete object that is referenced by a text block link
- delete object that is referenced by an attribute reference
- delete parent object of an object that is referenced by a link (either text or attribute or maybe even both)
@danielamormocea the link integrity feature works very well. I'd just have a few suggestions for the UX:
@danielamormocea nested folder structures are taken into account, which is good. Though, the system currently does not show the right breaches:
@danielamormocea the link integrity feature works very well. I'd just have a few suggestions for the UX:
In terms of UI, I have no better idea than to simply mention again before listing the items, that those are going to be deleted (although it seems redundant), or to add a separator again after the first list of items (but for me that looks uglier).
Furthermore, regarding the breaches, if there is a page inside a page that is referenced somewhere, the response won't give me that information, so the main page (the one selected) is the only "reference" I get, therefore the only page I can show to the user.
@tisto I also added the test for the reference, but unfortunately the linkintegrity api part does not detect objects referenced by a text block link
@danielamormocea I was suspecting that Plone and the restapi are the issue here. Then I guess we have to specify in plone.restapi how the backend is supposed to work and then work on the implementation there.
@danielamormocea would you be able to create a PR in plone.restapi documenting the missing functionality in a Python unit test?
FYI: Acceptance tests and code analysis are failing for this PR.
@tisto Acceptance tests are failing because there is no linkintegrity api for deleting items from contents (the failing tests are in content.js and folder-contents.js)
@danielamormocea I hope that now that we have the backend side, we are green and ready to merge.
@danielamormocea we have a test that fails, could you please take a look? Thanks!
@danielamormocea I fixed the Cypress test (it was using draftJS still). However, I can't see the @linkintegrity call happening... Could you please take a look?
I'd like to have this for Volto 16 which will happen next week for sure.
/cc @avoinea @alecghica
@avoinea I still couldn't take a look at this, it should work now but it keeps failing. Should be a silly thing.