Issue #13: Sort item list by when to buy urgency

[bbland1]

For an example of how to fill this template out, see this Pull Request.

Description

Adds a text indicator of the buying urgency status of an item to the right of the item and sorts the list based on the status. If an item is inactive it will be at the bottom of the list, overdue items will be at the top, and if any items have the same days until next purchase they will be additionally sorted alphabetically.

Related Issue

Closes #13

Acceptance Criteria

• Items in the list are shown with an indicator that tells the user they should buy the item “soon”, “kind of soon”, or “not soon”; or that the item is “inactive”
  • This urgency indicator does not rely on only color
• api/firestore.js exports a new comparePurchaseUrgency function with the following behaviors
  • sorts inactive items last, then
  • sorts items in ascending order of days until purchase, and
  • sorts items with the same days until purchase alphabetically
• Extend the functionality of comparePurchaseUrgency to sort “overdue” items to the top of the list
• Indicate in your UI when an item is overdue

Type of Changes

	Type
✓	💯 Enhancement
✓	✨ New Feature

Updates

Before

After

Testing Steps / QA Criteria

• Click the preview link for this PR
• Sign in with your account
• Click a list with item and navigate using the nav bar
• Items already on list should be sorted based on urgency status
• To see other status messages go to the firebase
• Open the list and item you want to change
• Change to various dates to see status change

[github-actions]

Visit the preview URL for this PR (updated for commit 6951bfd):

https://tcl-77-smart-shopping-list--pr35-bb-fz-sort-by-purcha-99ypueis.web.app

_{(expires Sun, 29 Sep 2024 16:41:46 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: b77df24030dca7d8b6561cb24957d2273c5e9d72}

[tannaurus]

I really like this! Did you consider an enum here? There's pros and cons:

• Enums are much easier to read/understand (and therefore produce more maintainable* code). You could have an enum that contains three variants: Priority.Low, Priority.Medium, Priority.High. Now you don't need to write down what -1 means. To some, that might be misunderstood to be very low priority. Careful documentation can get around that. You can also assign enums values (see some of the examples here) so you can still keep the numbers where you need them (if we're putting this in a database, for instance) but give them "more meaning" in the code.
• Numbers along are really great if you plan on adding more priorities in the future, which isn't really a thing here, but something to keep in mind. This is something I've seen done with errors before, where a function will have a big switch statement against every error variant a function could return, and assign a numerical value to the significance of that error. Assuming you want to trust someone else's evaluation of "significance" there, I don't think I would in production code but i'm just paranoid, you could decide to handle the "top 30% of significant errors" differently than "lower priority" errors. Imo this is typically the result of less-than-great api design and could be avoided in other ways, but I'll try to avoid a soap box here 😄

tldr; maybe consider enums with values assigned to them 🚀

[tannaurus]

🤦 I just realized this is for a sort - forget everything I said! Maybe we could leave a comment above this function explaining what it's used for and a brief overview of what it considers a "high priority" vs "low priority" ?

[tannaurus]

tiny suggestion that's damn near a nit pick

something I encourage is thinking a lot about where you initialize variables/perform logic inside "groupings" in logically a reasonable order. This function violates that a bit: daysUntilNextPurchase is calculated at the top, but it isn't' used until line 51 - this could mean that we're doing an operation that might have some performance cost that we never needed to "spend" (I don't think it's a big deal here, just something to consider generally).

Things like this also make code a bit harder to read. When reviewing logic, it's convenient to glance up a few lines (when possible) and see the value calculated that's then being compared in ifs

daysSinceItemLastActivity is another example of this. It might not ever be used if line 44 returns

[RossaMania]

PRAISE: I like the use of .sort() here.

[eternalmaha]

I agree! Great detail to implement the sorting even within the filtered list.

[eternalmaha]

I gotta pick your guys' brain a bit tomorrow about the logic of this piece of code :) I was trying to wrap my head around if comparePurchaseUrgency is only being called in the the action of filtering the search term, but of course that's not the case because the UI is being returned in the unfilteredList..

update:
Ahhh. I see that the filteredListItems is just checking if the searchTerm is included in the list then sorting the items based on urgency. I thought perhaps only items in the filtering of items by name, were being sorted.

[RossaMania]

I like this code.

Everything seems straightforward, and tied together nicely with that .sort() method!

[alex-andria]

Works great in testing it out and checking firebase, great work Falak and Brianna! 🙌 🙌 Added my comments but you knocked the logic for this out the park as I know this is a tricky one myself!

[alex-andria]

Nice work figuring out this logic!🌟🌟 It's a real brain twister at the start but this is very well done. know in the past others have explored switch functions for a cleaner look and can be faster to compile depending on the use case. Like Tanner said above this could also just be tightened up alternatively by turning some else if statements to if statements too.

[eternalmaha]

Wow!! Incredible work on the logic for this issue. When I was playing around with it, it was interesting to see the logic play out if I "purchased" an existing or new item earlier than expected (it immediately registering it as "overdue"). My suggestion for UI purposes, perhaps we could use a different word other than "overdue". I feel like it may confuse the user who might think they need to always buy the item immediately ? The other messages indicate a priority message ("soon", "kind of soon", "not soon"). Maybe along those lines, it could be "very soon" instead of "overdue"? My humble opinion!