Skip to content

Conversation

jazairi
Copy link
Contributor

@jazairi jazairi commented Mar 20, 2024

Why these changes are being introduced:

GDT requires an access to files filter, which allows users to facet based on access to geodata files.

Relevant ticket(s):

How this addresses that need:

This enables the filter in the same manner we've established for other filters:

  • Adds it to the the FILTER_PARAMS constants in the Enhancer and QueryBuilder models
  • Adds it to the nice_labels method
  • Includes it in the TIMDEX search heredoc

Side effects of this change:

  • There is an additional requirement in GDT-140 to render the filter as checkboxes. That introduces a fair amount of complexity into the view layer, so it made sense to make that change separate for ease of code review.
  • We likely don't want this filter to appear in non-GDT apps, so we should update the ACTIVE_FILTERS env in those apps accordingly before this lands.
  • The instructions for the ACTIVE_FILTERS env in the README have been updated for clarity.

Developer

  • All new ENV is documented in README
  • All new ENV has been added to Heroku Pipeline, Staging and Prod
  • ANDI or Wave has been run in accordance to
    our guide and
    all issues introduced by these changes have been resolved or opened as new
    issues (link to those issues in the Pull Request details above)
  • Stakeholder approval has been confirmed (or is not needed)

Code Reviewer

  • The commit message is clear and follows our guidelines
    (not just this pull request message)
  • There are appropriate tests covering any new functionality
  • The documentation has been updated or is unnecessary
  • The changes have been verified
  • New dependencies are appropriate or there were no changes

Requires database migrations?

NO

Includes new or updated dependencies?

NO

@mitlib mitlib temporarily deployed to timdex-ui-pi-gdt-140-ac-bg3q4v March 20, 2024 15:22 Inactive
@jazairi
Copy link
Contributor Author

jazairi commented Mar 20, 2024

As I mentioned in the commit message, this is not the final shape that this feature will take, but turning the filter into a form is likely to be a substantial PR in itself. I'm seeking to land the minimal version because it doesn't break anything, and it will hopefully make this and the subsequent PR easier to review.

@jazairi jazairi temporarily deployed to timdex-ui-pi-gdt-140-ac-bg3q4v March 20, 2024 15:54 Inactive
@coveralls
Copy link

coveralls commented Mar 20, 2024

Pull Request Test Coverage Report for Build 8363230630

Details

  • 3 of 3 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 98.872%

Totals Coverage Status
Change from base Build 8343451077: 0.0%
Covered Lines: 526
Relevant Lines: 532

💛 - Coveralls

@JPrevost JPrevost self-assigned this Mar 20, 2024
Copy link
Member

@JPrevost JPrevost left a comment

Choose a reason for hiding this comment

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

I really like this iterative approach. I personally feel like this is good enough for MVP and the next iteration could come in the future, but that's up to the project team to decide.

:shipit:

Why these changes are being introduced:

GDT requires an access to files filter, which allows users to
facet based on access to geodata files.

Relevant ticket(s):

* [GDT-140](https://mitlibraries.atlassian.net/browse/GDT-140)

How this addresses that need:

This enables the filter in the same manner we've established for
other filters:

* Adds it to the the FILTER_PARAMS constants in the Enhancer and
QueryBuilder models
* Adds it to the `nice_labels` method
* Includes it in the TIMDEX search heredoc

Side effects of this change:

* There is an additional requirement in GDT-140 to render the filter
as checkboxes. That introduces a fair amount of complexity into
the view layer, so it made sense to make that change separate for ease
of code review.
* We likely don't want this filter to appear in non-GDT apps, so we
should update the `ACTIVE_FILTERS` env in those apps accordingly
before this lands.
* The instructions for the `ACTIVE_FILTERS` env in the README have
been updated for clarity.
* Cassettes have been regenerated to account for the new filter.
@jazairi jazairi force-pushed the gdt-140-access-to-files-filter branch from 37a6817 to 05e1a84 Compare March 20, 2024 17:22
@jazairi jazairi merged commit 2f1f662 into main Mar 20, 2024
@jazairi jazairi deleted the gdt-140-access-to-files-filter branch March 20, 2024 17:24
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.

4 participants