Automatically update internal links

[david-venhoff]

Short description

This pr adds a feature to automatically update internal links when the content object they link to gets updated. This also works across regions. Right now no address is shown as link text for links to pois as requested in #1878, is that still wanted (And if so, do you have a preferred format @svenseeberg)? Also I still need to test if the performance is okay. Ideally I would like to do that on the test system, as the data are more realistic there.

Proposed changes

• When saving a page, fix all links in that page that are outdated
• When saving a page, find all links that point to this page, and for each link update the content translation of that link
• Add the attribute data-integreat-auto-update and automatically create a link text for those links if possible
• Add icons before event and poi links

Side effects

• When a user updates a page and if that causes another page to be updated, the author will be shown as the user who changed the other page, even if the user does not have permission to do so (Should alternatively no user be shown?)

Resolved issues

Fixes: #1878

---

Pull Request Review Guidelines

[codeclimate[bot]]

Code Climate has analyzed commit f75cee96 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 79.7% (50% is the threshold).

This pull request will bring the total coverage in the repository to 83.2% (0.0% change).

View more on Code Climate.

[seluianova]

I have started testing and got a couple questions:

1. Should it be the default icon or an icon set for the particular location/event ?

Add icons before event and poi links

2. Is it okay, that we add this attribute to external links as well?

<p><a class="link-external" href="https://hjkloj.com" data-integreat-auto-update="true">hjkloj.com</a></p>

3. Should the new label have the same style as the other labels? (smaller and grayed out).

4. Is it okay that the link text with an icon will be shown like this?

[david-venhoff]

Should it be the default icon or an icon set for the particular location/event ?

Seems like a good idea to me to use custom icons for events and pois

Is it okay, that we add this attribute to external links as well?

While technically redundant, they are ignored internally, so imo that is not a problem

[seluianova]

While technically redundant, they are ignored internally, so imo that is not a problem

I'm not insisting here, but perhaps it can be understood (by users) to mean that we will update the link text for external links as well (however I don't have any nice suggestions how to make it better)

[david-venhoff]

I have added the functionality to load custom defined icons, which looks like this:

I have also adjusted the behavior of the link dialog, so that either the text and url input can be edited, or that the 'link to internal content' input and the checkbox can be edited. Unfortunately, tinymce is very buggy and does not visually display which inputs are enabled and which are disabled. I also tried disable the checkbox on initial load, but that causes another tinymce bug where it will stay visually disabled, even when it gets enabled later… I feel like the current custom link input dialog is hard to maintain and the tinymce api is very limiting. Maybe we should create an issue to completely refactor it at some point?

4. Is it okay that the link text with an icon will be shown like this?

I think yes, because it accurately represents what the link looks like and users can expand the text field to view the full link text. If you don't agree, would you prefer just [image]title for links with both image and text?

[MariaKabanova]

I think it’s an awesome change which adds a lot of value for customers. I’ve tested this pull request, and I found a couple of small things.

The first one is about an icon. Looks like even though the link title and url are auto-updated, the icon of the internal link is not updated automatically. You can follow the next steps to reproduce that:

1. Open page, then insert the link on another page
2. Go to the linked page and change or delete an icon there
3. Return to the original page, where you can see that icon didn’t change. But if you click on “update”, the icon is changed after all

The second one contains the similar problem but in imprint. After inserting a link to the page in imprint, this link doesn’t automatically update. Steps to reproduce:

1. Open Imprint, then create the link to another page (insert/link)
2. Go to the linked page and change its title
3. Return to the imprint and see that the title didn’t change
4. Click on “update” and see that title has changed

I'm not sure it's worth fixing, please let me know what you think.

[david-venhoff]

@seluianova

To be honest I think it was better when the user could control the checkbox regardless of the link type.

I agree. I dropped the commits that limit the tinymce ui interactions for now.

@MariaKabanova Thanks for the review! I have pushed a commit to also update links when the icon changes.

The second one contains the similar problem but in imprint. After inserting a link to the page in imprint, this link doesn’t automatically update.

This is because we currently do not check links of imprints (#1627). This problem should fix itself automatically when we enable the link checker for imprints.

[MariaKabanova]

@david-venhoff

Today I've tested again this branch and now everything is great! It means that now we need to wait for #1627 is fixed. After that I'll retest imprints. :-)

[david-venhoff]

If you update only an icon of the page/event/location, it's not updated automatically (as it works with titles), but only if you click "Update" on the page, where the corresponding link is used

That should already be implement (in 98b3c47fe5d87aa359e2f2f5fa179b9962ddd8b1)

[seluianova]

That should already be implement (in https://github.com/digitalfabrik/integreat-cms/commit/98b3c47fe5d87aa359e2f2f5fa179b9962ddd8b1)

Right, I may have messed something up when testing. Now everything works smoothly!

[david-venhoff]

Quick status update: I would like to do some performance testing before merging this pr, so I'll block it on #2516

[deen13]

Hey @david-venhoff, yesterday in the issue grooming, we discussed the performance meta issues and reached a consensus that we do not want those to block issues. Since you already removed the blocked label, I wanted to ask if this is ready for merge? ☺️

[david-venhoff]

I tested this with production data and, as expected, the performance was really bad. I am currently working on improving the performance, so that this pr can be merged.

[david-venhoff]

@seluianova @MizukiTemma I have added an optimization to reduce the time it takes to save. On the production data saving a page now takes one or two seconds longer than without this pr, which I think is acceptable. Could you take a look at the changes?

[MizukiTemma]

@seluianova @MizukiTemma I have added an optimization to reduce the time it takes to save. On the production data saving a page now takes one or two seconds longer than without this pr, which I think is acceptable. Could you take a look at the changes?

Sorry for having overlooked this request 🙈 Could you rebase and resolve the conflicts? Then I will check this PR again 💪

[david-venhoff]

The pr is now rebased onto develop :)

[MizukiTemma]

@david-venhoff Thank you for rebasing 👍 I found only one thins to fix. Cannot resolve keyword 'slug' into field occurs when a link is added to the imprint. (Sorry, this is an extra work due to my recent PR adding Imprint to the link check 🙏 )

[david-venhoff]

Great catch!

[MizukiTemma]

@david-venhoff So cool 😻 Let's merge 😻

Just don't forget to squash them 😉