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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 10 commits into from
Sep 22, 2024
Merged

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

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
badabe5
created comparePurchaseUrgency function
bbland1 Sep 19, 2024
6d8fdd0
added urgency status to list item check box
bbland1 Sep 19, 2024
cece0fb
cleaned up pseudo code from firebase.ts and implemented overdue logic…
bbland1 Sep 19, 2024
9c0bf6f
update the shopping list api readme to include the new comparePurchas…
bbland1 Sep 19, 2024
091c3c7
update readme to be more descriptive of the objects used as parameters
bbland1 Sep 21, 2024
42dea27
removed buy from status for consistency, moved some grouping of logic
bbland1 Sep 21, 2024
694d7f8
cleaning up with guard clauses for the status checks added an inactiv…
bbland1 Sep 21, 2024
41e2ef2
Merge branch 'main' into bb-fz/sort-by-purchase-urgency
bbland1 Sep 22, 2024
b2e3fa9
Small spellckeck correction in readme
zahrafalak Sep 22, 2024
6951bfd
updated api readme to reflect the two daysSinceLastActivity
bbland1 Sep 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 37 additions & 9 deletions src/api/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@ The `firebase.js` file contains all of the functions that communicate with your

This function adds a new user to the database on their initial sign-in.

| Parameter | Type | Description |
| --------- | -------- | ---------------------------------------------------- |
| `user` | `object` | The user object returned by Firebase Authentication. |
| Parameter | Type | Description |
| ------------ | -------- | ----------------------------------------------------------------------- |
| `user.name` | `string` | The name of the user returned in an object by Firebase Authentication. |
| `user.email` | `string` | The email of the user returned in an object by Firebase Authentication. |
| `user.uid` | `string` | The uid of the user returned in an object by Firebase Authentication. |

### `createList`

Expand Down Expand Up @@ -46,24 +48,50 @@ This function takes user-provided data and uses it to create a new item in the F

#### Note

**`daysUntilNextPurchase` is not added to the item directly**. It is used alomngside the `getFutureDate` utility function to create a new _JavaScript Date_ that represents when we think the user will buy the item again.
**`daysUntilNextPurchase` is not added to the item directly**. It is used alongside the `getFutureDate` utility function to create a new _JavaScript Date_ that represents when we think the user will buy the item again.
bbland1 marked this conversation as resolved.
Show resolved Hide resolved

### `updateItem`

This function takes user-provided data and uses it to update an exiting item in the Firestore database.

| Parameter | Type | Description |
| ---------- | -------- | --------------------------------------------------------------- |
| `listPath` | `string` | The Firestore path of the list to which the item will be added. |
| `item` | `string` | The item object. |
| Parameter | Type | Description |
| ------------------------ | ----------------------------- | --------------------------------------------------------------------------------------- |
| `listPath` | `string` | The Firestore path of the list to which the item will be added. |
| `item.id` | `string` | A unique identifier of the item generated by firebase when it is added to the database. |
| `item.name` | `string` | The name of the item. |
| `item.dateLastPurchased` | `FirebaseTimestamp` or `null` | The date the item was last purchased, `null` when item first added. |
| `item.dateNextPurchased` | `FirebaseTimestamp` | The date the item is predicted to be purchased by next. |
| `item.totalPurchases` | `number` | The total number of times the item has been purchased. |
| `item.dateCreated` | `FirebaseTimestamp` | The date the item was added to the user's list. |

### `deleteItem`

🚧 To be completed! 🚧

### `comparePurchaseUrgency`

This function compares two item objects to determine their priority order for sorting. It evaluates `daysSineLastActivity` of `item1` and uses `daysSineLastActivity`, `item1.dateNextPurchased` and `item2.dateNextPurchased` to establish the order urgency.

| Parameter | Type | Description |
| ------------------------- | ----------------------------- | --------------------------------------------------------------------------------------- |
| `item1` | `object` | The first item being compared to the second item. |
| `item2` | `object` | The second item being compared to the first item. |
| `item1.id` | `string` | A unique identifier of the item generated by firebase when it is added to the database. |
| `item1.name` | `string` | The name of the item. |
| `item1.dateLastPurchased` | `FirebaseTimestamp` or `null` | The date the item was last purchased, `null` when item first added. |
| `item1.dateNextPurchased` | `FirebaseTimestamp` | The date the item is predicted to be purchased by next. |
| `item1.totalPurchases` | `number` | The total number of times the item has been purchased. |
| `item1.dateCreated` | `FirebaseTimestamp` | The date the item was added to the user's list. |
| `item2.id` | `string` | A unique identifier of the item generated by firebase when it is added to the database. |
| `item2.name` | `string` | The name of the item. |
| `item2.dateLastPurchased` | `FirebaseTimestamp` or `null` | The date the item was last purchased, `null` when item first added. |
| `item2.dateNextPurchased` | `FirebaseTimestamp` | The date the item is predicted to be purchased by next. |
| `item2.totalPurchases` | `number` | The total number of times the item has been purchased. |
| `item2.dateCreated` | `FirebaseTimestamp` | The date the item was added to the user's list. |

## The shape of the the data

When we request a shopping list, it is sent to us as an array of objects with a specific shape – a specific arrangeement of properties and the kinds of data those properties can hold. This table describes the shape of the items that go into the shopping list.
When we request a shopping list, it is sent to us as an array of objects with a specific shape – a specific arrangement of properties and the kinds of data those properties can hold. This table describes the shape of the items that go into the shopping list.

| Property | Type | Description |
| ------------------- | -------- | ----------------------------------------------------------------- |
Expand Down
58 changes: 58 additions & 0 deletions src/api/firebase.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,3 +301,61 @@ export async function deleteItem() {
* this function must accept!
*/
}

export function comparePurchaseUrgency(
item1: ListItem,
item2: ListItem,
): -1 | 0 | 1 {
const currentDate = new Date();

// Check for last time item1 had any activity
const daysSinceItem1LastActivity = item1.dateLastPurchased
? getDaysBetweenDates(currentDate, item1.dateLastPurchased.toDate())
: getDaysBetweenDates(currentDate, item1.dateCreated.toDate());

// De-prioritize an item if its had no activity for more than 60 days
if (daysSinceItem1LastActivity >= 60) {
return 1;
}

// Prioritize item1 if current date is past next purchase date and not inactive yet
if (currentDate > item1.dateNextPurchased.toDate()) {
return -1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

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 🚀

Copy link
Collaborator

Choose a reason for hiding this comment

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

🤦 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" ?

}

// Check for last time item2 had any activity
const daysSinceItem2LastActivity = item2.dateLastPurchased
? getDaysBetweenDates(currentDate, item2.dateLastPurchased.toDate())
: getDaysBetweenDates(currentDate, item2.dateCreated.toDate());

// Prioritize item2 if current date is past next purchase date and not inactive yet
if (
currentDate > item2.dateNextPurchased.toDate() &&
daysSinceItem2LastActivity < 60
) {
return 1;
}

const item1DaysUntilNextPurchased = getDaysBetweenDates(
currentDate,
item1.dateNextPurchased.toDate(),
);
const item2DaysUntilNextPurchased = getDaysBetweenDates(
currentDate,
item2.dateNextPurchased.toDate(),
);

//Compare days until next purchase for item1 and item2
//if item1 needs to be purchased sooner, prioritize item1 over item2
if (item1DaysUntilNextPurchased < item2DaysUntilNextPurchased) {
return -1;
}

//if item2 needs to be purchased sooner, prioritize item2 over item1
if (item1DaysUntilNextPurchased > item2DaysUntilNextPurchased) {
return 1;
}

//if both items have the same sort order, we sort alphabetically
return item1.name.toLowerCase() < item2.name.toLowerCase() ? -1 : 1;
}
1 change: 1 addition & 0 deletions src/components/ListItem.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
display: flex;
flex-direction: row;
font-size: 1.2em;
justify-content: space-between;
}

.ListItem-checkbox {
Expand Down
34 changes: 33 additions & 1 deletion src/components/ListItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import "./ListItem.css";
import { updateItem, ListItem } from "../api";
import { useState } from "react";
import toast from "react-hot-toast";
import { moreThan24HoursPassed } from "../utils";
import { moreThan24HoursPassed, getDaysBetweenDates } from "../utils";

interface Props {
item: ListItem;
Expand All @@ -29,6 +29,37 @@ export function ListItemCheckBox({ item, listPath }: Props) {
? !moreThan24HoursPassed(item.dateLastPurchased.toDate())
: false;

const getUrgencyStatus = (item: ListItem) => {
const currentDate = new Date();

const daysUntilNextPurchase = getDaysBetweenDates(
currentDate,
item.dateNextPurchased.toDate(),
);

const daysSinceItemLastActivity = item.dateLastPurchased
? getDaysBetweenDates(currentDate, item.dateLastPurchased.toDate())
: getDaysBetweenDates(currentDate, item.dateCreated.toDate());

if (daysSinceItemLastActivity >= 60) {
bbland1 marked this conversation as resolved.
Show resolved Hide resolved
return "inactive";
}

if (currentDate > item.dateNextPurchased.toDate()) {
return "overdue";
}

if (daysUntilNextPurchase >= 30) {
return "not soon";
}

if (daysUntilNextPurchase <= 7) {
return "soon";
}

return "kind of soon";
};

const handleCheckChange = async (e: React.ChangeEvent<HTMLInputElement>) => {
const newCheckedState = e.target.checked;

Expand Down Expand Up @@ -67,6 +98,7 @@ export function ListItemCheckBox({ item, listPath }: Props) {
/>
{item.name}
</label>
<span>{getUrgencyStatus(item)}</span>
</div>
);
}
10 changes: 6 additions & 4 deletions src/views/authenticated/List.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useState, useMemo } from "react";
import { ListItemCheckBox } from "../../components/ListItem";
import { FilterListInput } from "../../components/FilterListInput";
import { ListItem } from "../../api";
import { ListItem, comparePurchaseUrgency } from "../../api";
import { useNavigate } from "react-router-dom";

interface Props {
Expand All @@ -14,9 +14,11 @@ export function List({ data: unfilteredListItems, listPath }: Props) {
const [searchTerm, setSearchTerm] = useState<string>("");

const filteredListItems = useMemo(() => {
return unfilteredListItems.filter((item) =>
item.name.toLowerCase().includes(searchTerm.toLowerCase()),
);
return unfilteredListItems
.filter((item) =>
item.name.toLowerCase().includes(searchTerm.toLowerCase()),
)
.sort(comparePurchaseUrgency);
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

@eternalmaha eternalmaha Sep 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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.

}, [searchTerm, unfilteredListItems]);

// Early return if the list is empty
Expand Down