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

Tidy up organisation lists #3215

Merged
merged 3 commits into from
Oct 9, 2024
Merged

Tidy up organisation lists #3215

merged 3 commits into from
Oct 9, 2024

Conversation

yndajas
Copy link
Member

@yndajas yndajas commented Oct 7, 2024

Trello

  • Split organisations index into two tabs, one for active organisations and one for closed organisations (similar to the applications index)
  • Sort active organisations ahead of closed ones (this affects the users index filter options

Screenshots

Before

image image

After

image image image

This application is owned by the publishing platform team. Please let us know in #govuk-publishing-platform when you raise any PRs.

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

This brings the spec more in line with others. It also removes some
spec-level setup, since it was only used in a couple of tests - setting
it up before every tests feels inefficient
... one for active organisations, one for inactive. This will reduce the
closed organisation noise and make it easier to look through active
organisations by default. We'll separately set a default order on
organisations so that closed ones come last, but this felt like a nicer
way to handle the organisations index, and follows the pattern
established by the applications index

This required a change to a 2SV integration test so that it looks for a
table within the active tab/container, since both tabs and hence both
tables are within the underlying HTML. Initially I looked to extend
existing methods in the `HtmlTableHelpers` file to allow us to look for
a table within the right container, but I noticed that the helper's
methods were only used in this integration test. Since their use case
was specific to this spec, I've opted to reduce the abstraction here and
keep it simple, similar to recent minor shifts in the approach to access
and permissions integration tests. This reduces complexity in the
codebase, and brings all the relevant code closer to the context of the
tests - I'm not always a proponent of this approach, but it felt
appropriate here given the limited use case!
Given all the organisation autocompletes filter out closed organisations
and the organisations index is now tabbed, the closed ordering might
only affect the users index filter at present. We could alternatively
just sort manually in this use case

Ordering by a Boolean field seemed a little tricky, but this helepd:
https://stackoverflow.com/a/12334676/4002016
Copy link
Contributor

@JonathanHallam JonathanHallam left a comment

Choose a reason for hiding this comment

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

This is looking great, thank you!

@yndajas yndajas merged commit 03aa16e into main Oct 9, 2024
15 checks passed
@yndajas yndajas deleted the tidy-up-organisation-lists branch October 9, 2024 14:18
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