Skip to content

Debates filtering#6438

Merged
tramuntanal merged 6 commits intodevelopfrom
feature/debates-filtering
Sep 24, 2020
Merged

Debates filtering#6438
tramuntanal merged 6 commits intodevelopfrom
feature/debates-filtering

Conversation

@oriolgual
Copy link
Copy Markdown
Contributor

@oriolgual oriolgual commented Aug 27, 2020 •
edited
Loading

🎩 What? Why?

Adds more filters to debate search. Also, refactored and extracted common search logic and created shared examples for tests.

📌 Related Issues

📋 Subtasks

  • Add CHANGELOG upgrade notes, if required
  • If there's a new public field, add it to GraphQL API
  • Add documentation regarding the feature
  • Add/modify seeds
  • Add tests
  • Another subtask

📷 Screenshots

Screenshot 2020-08-26 at 10 15 36
Screenshot 2020-08-26 at 10 15 57
Screenshot 2020-08-26 at 10 16 12
Screenshot 2020-08-26 at 10 16 24
Screenshot 2020-08-26 at 10 16 35
Screenshot 2020-08-26 at 10 16 50
Screenshot 2020-08-26 at 10 17 02
image

@oriolgual oriolgual force-pushed the feature/debates-filtering branch from ea636f8 to 43bb631 Compare August 27, 2020 09:06
oriolgual
oriolgual commented Aug 27, 2020
View reviewed changes
Comment thread decidim-core/lib/decidim/filterable.rb Outdated
Comment thread decidim-debates/app/helpers/decidim/debates/application_helper.rb
Comment thread decidim-debates/app/helpers/decidim/debates/application_helper.rb
Comment thread decidim-proposals/app/helpers/decidim/proposals/proposals_helper.rb
Decidim::CheckBoxesTreeHelper::TreePoint.new("accepted", t("decidim.proposals.application_helper.filter_state_values.accepted")),
Decidim::CheckBoxesTreeHelper::TreePoint.new("evaluating", t("decidim.proposals.application_helper.filter_state_values.evaluating")),
Decidim::CheckBoxesTreeHelper::TreePoint.new("not_answered", t("decidim.proposals.application_helper.filter_state_values.not_answered")),
Decidim::CheckBoxesTreeHelper::TreePoint.new("state_not_published", t("decidim.proposals.application_helper.filter_state_values.not_answered")),
Copy link
Copy Markdown
Contributor Author

@oriolgual oriolgual Aug 27, 2020

Choose a reason for hiding this comment

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

This was renamed to match the scope at the model.

Comment thread decidim-proposals/app/models/decidim/proposals/proposal.rb Outdated
where.not(coauthorships_count: 0)
.joins(:coauthorships)
.where.not(decidim_coauthorships: { decidim_author_type: "Decidim::Organization" })
.where(decidim_coauthorships: { decidim_user_group_id: nil })
Copy link
Copy Markdown
Contributor Author

@oriolgual oriolgual Aug 27, 2020

Choose a reason for hiding this comment

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

Otherwise, it also includes proposals created by user groups.

@oriolgual oriolgual force-pushed the feature/debates-filtering branch 2 times, most recently from 5d38e93 to 52d4ef0 Compare August 27, 2020 11:55
@oriolgual oriolgual linked an issue Aug 27, 2020 that may be closed by this pull request
Improve the Debates component: Listing #6003
Closed
10 tasks
@oriolgual oriolgual force-pushed the feature/debates-filtering branch 2 times, most recently from 2975fb0 to 3c07ca9 Compare September 1, 2020 10:59
@oriolgual oriolgual marked this pull request as ready for review September 1, 2020 11:15
@tramuntanal
Copy link
Copy Markdown
Contributor

tramuntanal commented Sep 7, 2020

@oriolgual there are some conflicts to be solved..

@oriolgual oriolgual force-pushed the feature/debates-filtering branch 4 times, most recently from 1d6cc83 to f4bd2e4 Compare September 8, 2020 10:34
@github-actions github-actions bot mentioned this pull request Sep 8, 2020
Closed
@oriolgual oriolgual force-pushed the feature/debates-filtering branch 3 times, most recently from bfcc59f to 3ce5356 Compare September 9, 2020 19:19
@oriolgual
Copy link
Copy Markdown
Contributor Author

oriolgual commented Sep 10, 2020

Finally everything green! Could you please review it @tramuntanal?

@github-actions github-actions bot mentioned this pull request Sep 10, 2020
Closed
@carolromero carolromero added the release: v0.23 Issues that need to be tackled for v0.23 label Sep 14, 2020
@tramuntanal
Copy link
Copy Markdown
Contributor

tramuntanal commented Sep 15, 2020

I've resolved a trivial conflict in projects_controller

@tramuntanal tramuntanal self-assigned this Sep 15, 2020
@oriolgual
Copy link
Copy Markdown
Contributor Author

oriolgual commented Sep 15, 2020

@tramuntanal I actually deleted that part of the code from projects_controller because I moved it to decidim-core/app/controllers/concerns/decidim/filter_resource.rb, let me double check the changes.

@oriolgual oriolgual force-pushed the feature/debates-filtering branch from ed0211b to 3e43348 Compare September 15, 2020 07:49
@oriolgual
Copy link
Copy Markdown
Contributor Author

oriolgual commented Sep 15, 2020

Fixed!

tramuntanal
tramuntanal suggested changes Sep 15, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@tramuntanal tramuntanal left a comment

Choose a reason for hiding this comment

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

Hi @oriolgual great refactor and performance improvements via counter caches!

I've left a few minor comments to be clarified and that's all 😄

Comment thread decidim-proposals/db/migrate/20170118120151_add_counter_cache_votes_to_proposals.rb Outdated
Comment thread decidim-comments/app/models/decidim/comments/comment.rb
Comment thread decidim-comments/lib/decidim/comments/commentable.rb
Comment thread decidim-debates/app/models/decidim/debates/debate.rb Outdated
Comment thread decidim-debates/app/helpers/decidim/debates/application_helper.rb Outdated
Comment thread decidim-debates/lib/decidim/debates/test/factories.rb
Comment thread decidim-dev/lib/decidim/dev/test/rspec_support/capybara.rb Outdated
@oriolgual oriolgual force-pushed the feature/debates-filtering branch from 3e43348 to cfcb6b4 Compare September 15, 2020 14:51
@oriolgual
Copy link
Copy Markdown
Contributor Author

oriolgual commented Sep 15, 2020

@tramuntanal everything addressed, thanks for the review!

@oriolgual oriolgual force-pushed the feature/debates-filtering branch from cfcb6b4 to 67feffc Compare September 15, 2020 15:13
@oriolgual
Copy link
Copy Markdown
Contributor Author

oriolgual commented Sep 21, 2020

@tramuntanal could you review this please?

@tramuntanal
Copy link
Copy Markdown
Contributor

tramuntanal commented Sep 24, 2020

ok @oriolgual I'm accepting the PR to not block the PAM project but how did you manage to make tests succeed, simply by re-runnig failing workflows?

@tramuntanal tramuntanal merged commit db9afbe into develop Sep 24, 2020
@tramuntanal tramuntanal deleted the feature/debates-filtering branch September 24, 2020 10:24
@oriolgual
Copy link
Copy Markdown
Contributor Author

oriolgual commented Sep 24, 2020

ok @oriolgual I'm accepting the PR to not block the PAM project but how did you manage to make tests succeed, simply by re-runnig failing workflows?

Yup, since the errors where from seg faults.

This was referenced Sep 28, 2020
Merged
Merged
@oliverbarnes oliverbarnes mentioned this pull request Mar 16, 2021
Closed
@ahukkanen ahukkanen mentioned this pull request Mar 16, 2021
Merged
12 tasks
@leio10 leio10 mentioned this pull request Jul 16, 2021
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@tramuntanal tramuntanal tramuntanal approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@tramuntanal tramuntanal

Labels

in-review module: accountability module: blogs module: budgets module: comments module: core module: debates module: dev module: meetings module: proposals module: sortitions release: v0.23 Issues that need to be tackled for v0.23

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Improve the Debates component: Listing

3 participants

@oriolgual @tramuntanal @carolromero