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

Align UI groups filtering with the rest of decidim #8105

Merged
merged 2 commits into from
Sep 6, 2021

Conversation

alecslupu
Copy link
Contributor

@alecslupu alecslupu commented Jun 5, 2021

🎩 What? Why?

Currently, the user groups page is displaying a one of a type filtering UI, and this PR aims to fix the issue, while adding also sorting options for groups module.
It replaces the
image

with the following information:

image

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • AVOID breaking the continuous integration build.
  • AVOID making significant changes to the overall architecture.

📷 Screenshots

Please add screenshots of the changes you're proposing
image

♥️ Thank you!

@alecslupu alecslupu force-pushed the ale-user-groups-refactor branch 2 times, most recently from 1d9a471 to 91660e1 Compare June 5, 2021 06:59
@andreslucena
Copy link
Member

Hello @alecslupu
Can you provide an explanation of what you're doing here? (With a better title and description would be awesome too 😄)
As far as I understand this is making the UserGroups filtering consistent (in code and UI) with the rest of the admin filters. If that's it I think we can live without the Metadecidim proposal link 😉

@alecslupu alecslupu changed the title Ale user groups refactor Align UI groups filtering with the rest of decidim Jul 12, 2021
@alecslupu alecslupu marked this pull request as ready for review July 12, 2021 11:42
@alecslupu
Copy link
Contributor Author

@andreslucena This PR is doing exactly what you have mentioned. I had it left in Draft state as i was not able to find time to fill in the description.

@leio10, this can be reviewed.

@andreslucena
Copy link
Member

I couldn't actually try this locally as I have an error when regenerating the development_app:

ActionView::Template::Error: Webpacker configuration file not found

Although this isn't critical as this was solved already in develop

@alecslupu alecslupu force-pushed the ale-user-groups-refactor branch from 91660e1 to ac0ac6b Compare July 13, 2021 06:51
@alecslupu
Copy link
Contributor Author

@andreslucena I just rebased the current branch with decidim/develop branch.

@andreslucena
Copy link
Member

@andreslucena I just rebased the current branch with decidim/develop branch.

Awesome, thanks @alecslupu. I've tried this locally and found a bug:

When clicking in the State column it doesn't sort correctly.

imatge

Could you please check that out? Thanks

@alecslupu
Copy link
Contributor Author

@carolromero , @andreslucena This can be tested on http://alecslupu.go.ro:3309/ (default decidim admin credentials)

@andreslucena
Copy link
Member

@carolromero , @andreslucena This can be tested on http://alecslupu.go.ro:3309 (default decidim admin credentials)

Ok, I tested this and I've detected a bug that I see it wasn't solved. Sorting by the state doesn't work. Can you please check this out @alecslupu 🙏🏽 ? Thanks

@alecslupu alecslupu force-pushed the ale-user-groups-refactor branch from ac0ac6b to aa50bd8 Compare July 19, 2021 09:29
@alecslupu
Copy link
Contributor Author

@andreslucena the fixes have been put in place.

@andreslucena
Copy link
Member

@andreslucena the fixes have been put in place.

Awesome, it works correctly 🚀 🚀

@leio10 leio10 added in-review module: admin type: feature PRs or issues that implement a new feature labels Jul 19, 2021
Copy link
Contributor

@leio10 leio10 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! 👏
Please @alecslupu, can you add some tests for the ordering and filtering of user groups? Thanks!

@alecslupu alecslupu force-pushed the ale-user-groups-refactor branch from aa50bd8 to 570c987 Compare September 5, 2021 18:58
@alecslupu alecslupu force-pushed the ale-user-groups-refactor branch from 570c987 to 27f43a5 Compare September 5, 2021 19:02
@alecslupu alecslupu requested a review from leio10 September 5, 2021 19:03
Copy link
Contributor

@leio10 leio10 left a comment

Choose a reason for hiding this comment

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

Awesome @alecslupu!! LGTM!

@leio10 leio10 merged commit 1835e1d into decidim:develop Sep 6, 2021
@alecslupu alecslupu deleted the ale-user-groups-refactor branch September 30, 2021 16:48
@alecslupu alecslupu added this to the 0.26.0 milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-review module: admin type: feature PRs or issues that implement a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants