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

Filter cycle specific labels on inventory list #3858

Merged
merged 14 commits into from
Mar 20, 2023

Conversation

perryr16
Copy link
Contributor

@perryr16 perryr16 commented Feb 13, 2023

Any background context you want to provide?

The new Inventory List page's filter auto-complete shows all labels that are associated with any property, in any cycle. This differs from the functionality on Inventory List (legacy) page where only labels associated with the cycle's inventory are present.

What's this PR do?

  • Adds cycle-specific inventory filters to the returned labels. The prior functionality returned all labels that have been applied to a property regardless of cycle.
  • Changes the order of function calls. For context, the filter labels function generated labels and then found the cycle's inventory. However, to accurately filter labels the inventory ($scope.data) must first exist.

How should this be manually tested?

  • create at least 2 cycles.
  • apply cycle specific labels to a few properties
  • apply cross-cycle labels to a few properties
  • click the "Filter by Label" input field. The auto-complete should only include labels applied to this cycle.

What are the relevant tickets?

#3818

Screenshots (if appropriate)

NEW
2 cycles with unique and common labels.
Screenshot 2023-02-13 at 3 02 03 PM
Screenshot 2023-02-13 at 3 00 55 PM

OLD
Screenshot 2023-02-13 at 3 07 20 PM

@perryr16 perryr16 added the Bug label Feb 13, 2023
@perryr16 perryr16 changed the title load_inventory should call get_labels, adds inventory specific filtering Filter cycle specific labels on inventory list Feb 13, 2023
@perryr16 perryr16 marked this pull request as ready for review February 14, 2023 15:17
@haneslinger
Copy link
Contributor

I found a small bug:

Says I have a filter group, "Called", which simply filters fro properties with the label "Call"
If I go to a cycle where none of the properties have the label Call and apply filter group Call, it'll break.
This is because "Call" was filtered out of labels (as no property has the label call), and the UI got confused.

@axelstudios axelstudios force-pushed the 3818-Filter-labels-by-cycle-inventory branch from 1ceb236 to 4736b8c Compare March 20, 2023 06:14
@axelstudios
Copy link
Member

This PR had two regressions:

  • Changing the label filter had no effect until another filter was modified
  • It only suggested labels for the current page rather than the current cycle. If a record on a different page had a different label it wouldn't appear as an option

It also doubled the inventory fetch requests on change.

@perryr16 I added some fixes:

  • When requesting labels for property_views or taxlot_views, cycle_id is now an optional path argument to limit is_applied ids to the selected cycle
    • This results in a decent performance improvement since listing labels previously required looping over every view to check is_applied, rather than just views from the chosen cycle
  • When changing cycles, labels that aren't applicable are removed from the filter

@axelstudios axelstudios merged commit 76585bd into develop Mar 20, 2023
@axelstudios axelstudios deleted the 3818-Filter-labels-by-cycle-inventory branch March 20, 2023 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants