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

Update internal links when cloning regions #2126

Merged
merged 3 commits into from
Mar 23, 2023

Conversation

timobrembeck
Copy link
Member

Short description

This adds a few utils for link replacements and applies the replacement mechanism on cloned regions.

Proposed changes

  • Add utility function to replace links in content
  • Update internal links when cloning regions
  • Add management command to replace links in content

Side effects

Well, testing this was quite a journey. The main problem was that our normal test cases not not allow interactions with the database, and I noticed very weird side effects from enabling transaction support only for few test cases.
In particular, the problem was that the transaction test cases roll back not only the changes from the particular test case, but also remove the test data including the roles that are generated during the migrations.
I think I now got around them by adding the extension pytest-order and telling pytest it should execute the transaction test cases last. Also, I needed to provide another test fixture just for roles to make sure I can re-populate the database after it has been rolled back by another transaction test case.

Resolved issues

Fixes: #1383


Pull Request Review Guidelines

@timobrembeck timobrembeck requested a review from a team as a code owner March 16, 2023 16:07
@codeclimate
Copy link

codeclimate bot commented Mar 16, 2023

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

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

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

View more on Code Climate.

Copy link
Contributor

@JoeyStk JoeyStk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you a lot! This is a very important feature!
To be honest this is a little bit above my head. I still tried my best and for me everything seemed to work, but it would be good if someone else could have a closer look

Copy link
Member

@MizukiTemma MizukiTemma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for PR 😃 This is a very nice feature!
Link replace works as expected during region cloning and by command.

I just noticed one case (this is not your fault, though).
When the source regions has no impressum, replace_links will not be run. In this case an attribute error will be shown but a new region will be created with not updated links. I think this happens with source reagions without page (or language_tree) too.

How to reproduce:

  1. Go to the region Nürnberg (default data, no impressum).
  2. Put some internal links in a page.
  3. Clone Nürnberg, entounter to an attribute error.
  4. Return and see unupdated links in the page in the new region.

Please dismiss this if an error page is enough or we can assume that a source region has always language_tree, page and impressum.

@timobrembeck
Copy link
Member Author

@MizukiTemma Thanks a lot for the report, since this is a separate problem that already exists on the develop branch, I opened issue #2153 for this and fixed it in #2154.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update internal links when cloning regions
3 participants