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 proposals advanced search #2133

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Update proposals advanced search #2133

wants to merge 8 commits into from

Conversation

Senen
Copy link

@Senen Senen commented May 7, 2021

References

This is part of the Ciudades Abiertas project.

Objectives

Improve proposals search:

  • Remove the author category filter from the advanced search form
  • Add combo box filter by geozone (district)
  • Show proposals geozone tag at index page previously to tags.
  • Reduce the maximum number of tags to display by one.
  • Add order option to show oldest proposals first
  • Update the proposals map page geozone links and image area links to use the filter by geozone.

Visual Changes

Before

Advanced search filter with filter by author category
Captura de pantalla 2021-05-13 a las 17 40 48

How the proposal was shown before this change
Captura de pantalla 2021-05-13 a las 17 40 57

After

Advanced search filter
Captura de pantalla 2021-05-13 a las 17 37 34

Show proposals district as a tag
Captura de pantalla 2021-05-13 a las 17 37 46

Does this PR need a Backport to CONSUL?

No

Copy link

@javierm javierm left a comment

Choose a reason for hiding this comment

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

Here's an initial review; let me know what you think! 😉

app/views/custom/proposals/_advanced_search.html.erb Outdated Show resolved Hide resolved
spec/features/proposals_spec.rb Outdated Show resolved Hide resolved
spec/features/proposals_spec.rb Outdated Show resolved Hide resolved
app/helpers/proposals_helper.rb Outdated Show resolved Hide resolved
app/helpers/proposals_helper.rb Outdated Show resolved Hide resolved
app/models/custom/proposal.rb Outdated Show resolved Hide resolved
app/controllers/proposals_controller.rb Outdated Show resolved Hide resolved
app/views/custom/proposals/_geozone.html.erb Outdated Show resolved Hide resolved
app/views/custom/proposals/_geozone.html.erb Outdated Show resolved Hide resolved
app/views/custom/proposals/map.html.erb Outdated Show resolved Hide resolved
The 0.3.5 version was yanked due to license issues.

More information at:
rails/rails#41750
@Senen Senen changed the base branch from executions-tab-management to master May 13, 2021 13:03
@Senen Senen self-assigned this May 13, 2021
Copy link

@javierm javierm left a comment

Choose a reason for hiding this comment

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

@Senen Only one comment this time. Let me know what you think 😉.

app/views/custom/proposals/_advanced_search.html.erb Outdated Show resolved Hide resolved
@Senen Senen force-pushed the proposals_filters branch 2 times, most recently from b2ddc0e to cd5b265 Compare May 20, 2021 09:59
@Senen Senen marked this pull request as ready for review May 20, 2021 10:25
Copy link

@javierm javierm left a comment

Choose a reason for hiding this comment

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

A couple of minor comments regarding the tests. Feel free to ignore them 😉.

spec/features/proposals_spec.rb Outdated Show resolved Hide resolved
spec/features/proposals_spec.rb Outdated Show resolved Hide resolved
spec/features/proposals_spec.rb Outdated Show resolved Hide resolved
Senen added 7 commits May 29, 2021 11:47
* Add combo box with geozones to advanced search
* Add scope to custom proposal model to filter records by geozone
These specs check that the advanced search form works when
using multiple filters at a time. Now we removed the author category
filter we can use the new filter by geozone to check the same.
As we are going to modify how and where the geozone links are rendered.
As we want the proposals geozones links to point to the new combo box
within the advanced search.

We needed to remove the id attribute from the new custom geozone view as
we now render it at the same page many times. Otherwise the generated HTML
would be invalid.

No additional test are needed as the new feature behaves exactly the same
than the previous version but using the geozone filter instead of the search
by text.
Now we want the geozone links and the map image areas to point
to the proposals advanced search filter by geozone.

No additional test are needed as the feature keep working the same
it was with the previous version but using the advanced search form
filter by geozone instead of the search by plain text.
Copy link

@javierm javierm left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍.

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.

2 participants