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.

Sign up for GitHub

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

Jump to bottom

Backport 'Fix moderations for comments that are mapped to deleted resources' to v0.26 #9941

Merged
merged 4 commits into from Nov 8, 2022
Merged

Backport 'Fix moderations for comments that are mapped to deleted resources' to v0.26 #9941

merged 4 commits into from Nov 8, 2022

Conversation

andreslucena
Copy link
Member

🎩 What? Why?

Backport #9621 to v0.26

♥️ Thank you!

@ahukkanen @andreslucena
Fix moderations for comments that are mapped to deleted resources
8f61afa 
* Return nil for reported content url if the commentable is deleted

* Show "deleted resource" instead of the visit URL when URL is not returned

* Add system specs testing that moderations for comments with deleted resources work

* Fix issue hiding comments for deleted debates

* Add moderation test to hide comments for deleted resources

* Add manage moderations shared examples for debates

* Fix specs after merging with develop
@andreslucena andreslucena added backport Pull Requests that are a backport for a fixed bug module: admin module: comments type: fix PRs that implement a fix for a bug labels Oct 26, 2022
ahukkanen
ahukkanen requested changes Nov 1, 2022
View reviewed changes
Copy link
Contributor

@ahukkanen ahukkanen left a comment

Choose a reason for hiding this comment

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

There are broken specs in this. I think something is working slightly differently in 0.26.

@andreslucena
Copy link
Member Author

@ahukkanen I've been researching this one and what I found is that this is related to #8774, as that's the PR that introduced Capybara.server_port to specs. This is on develop and release/0.27-stable but isn't in release/0.26-stable

The problem is that as far as I see, it's not that easy as backporting that, as there are lots of specs using Capybara.server_port in develop:

$ grep -r Capybara.server_port decidim*/spec/ |wc -l
36

So, for now I propose to just drop these references in the backports, as that's the behavior of the specs on this version (v0.26).

As an alternative we could backport #9661 (the last iteration of Capybara.server_port with a fix with multiple parallel ports), but I'm not 100% sure if this will not introduce other problems. We should probably sync the 36 specs that are using for instance, but I don't know if that could be a rabbit hole.

FYI we have this same problem at other PRs (like #9947).

@ahukkanen
Copy link
Contributor

So, for now I propose to just drop these references in the backports, as that's the behavior of the specs on this version (v0.26).

Yes, I think this is the correct solution.

@ahukkanen ahukkanen merged commit d852e36 into release/0.26-stable Nov 8, 2022
@ahukkanen ahukkanen deleted the backport/0.26/fix-moderations-for-comments-t-9621 branch November 8, 2022 19:13
@andreslucena andreslucena mentioned this pull request Nov 9, 2022
Merged
@microstudi microstudi mentioned this pull request Feb 2, 2023
Closed
@alecslupu alecslupu added this to the 0.26.4 milestone Jul 14, 2023
@alecslupu alecslupu mentioned this pull request Jul 19, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahukkanen ahukkanen ahukkanen approved these changes

Assignees
No one assigned
Labels
backport Pull Requests that are a backport for a fixed bug module: admin module: comments type: fix PRs that implement a fix for a bug
Projects
None yet
Milestone
0.26.4
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@andreslucena @ahukkanen @alecslupu