-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Hide related content proposals when these proposals are retired by their authors #4196
Hide related content proposals when these proposals are retired by their authors #4196
Conversation
@matisnape Thanks for the pull request. Before reviewing, could you have a look at the failing tests? |
@javierm Will do, of course.
|
@matisnape That's strange 🤔. Which Ruby version are you using? What's the content of the |
@javierm Yeah, I have 2.5.8 like in the readme. I think last year I had the same issue and just watched the tests touching the area I was working with. |
That's strange, because the error you mentioned happens when using Ruby 2.6+ but doesn't happen with Ruby 2.5 or below. Anyway, I'll review the changes now that the tests pass. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matisnape Nice! I completely misunderstood what the issue was about even though I read it several times 😅, but you got it right 👍.
Regarding the tests you mentioned, I think a test in the spec/models/related_content_spec.rb
file should do the trick once we've moved the code to the Relationable
concern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I'll do some final tests on Monday and merge if everything is OK.
Thanks a lot!
References
#3559
Objectives
Hide retired proposals from "related content" section for show view for: debate, budget investment, proposal
Visual Changes
No changes
Notes
Not applicable