Mm qg predict next purchase #27
Conversation
|
Visit the preview URL for this PR (updated for commit 0d8b397): https://tcl-79-smart-shopping-list--pr27-mm-qg-predict-next-p-eticpkkj.web.app (expires Sun, 22 Sep 2024 12:45:53 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: d91d9ddbda780208241c52942f544acf8e81407a |
There was a problem hiding this comment.
Amazing work handling this issue, well done!!! I left a couple of adjustments that should be tackled before we can merge this PR.
Also, I noticed that you're fetching the item data inside the updateItem function, which isn't necessary. Since we already have the latest data from the useShoppingListData custom hook, it would be more efficient to pass that data down to the Item component and then into the updateItem function. This way, we avoid redundant fetches, making fewer API calls, which is always better for performance.
src/api/firebase.js
Outdated
|
|
||
| if (checked) { | ||
| await updateDoc(itemRef, { | ||
| dateLastPurchased: new Date(), | ||
| dateLastPurchased: Timestamp.fromDate(new Date()), |
There was a problem hiding this comment.
I believe it's enough to just have new Date() or the today variable you defined
src/api/firebase.js
Outdated
| dateNextPurchased: Timestamp.fromMillis( | ||
| today.setDate(today.getDate() + estimate), | ||
| ), |
There was a problem hiding this comment.
you can use the helper function getFutureDate that you already have to convert the estimate to a javascript date
src/api/firebase.js
Outdated
| const dateLastPurchasedJavaScriptObject = data.dateLastPurchased | ||
| ? data.dateLastPurchased.toDate() | ||
| : today; |
There was a problem hiding this comment.
If the item has never been purchased, we should use the date it was created at instead of today
src/components/ListItem.jsx
Outdated
| @@ -13,7 +13,7 @@ export function ListItem({ name, listPath, id, isChecked, datePurchased }) { | |||
|
|
|||
| useEffect(() => { | |||
| const today = new Date().getTime(); | |||
| const datePurchasedInMillis = datePurchased?.toMillis(); | |||
| const datePurchasedInMillis = datePurchased; | |||
There was a problem hiding this comment.
seems like the ?.toMillis that got deleted here is causing the items to uncheck whenever the page refreshes, which is not the desired behavior
Description
When a new purchase is recorded the next purchase date is automatically computed and the app learn how often the item is purchased.
Related Issue
Closes #11
Acceptance Criteria
dateNextPurchasedproperty is calculated using thecalculateEstimatefunction and saved to the Firestore databasedateNextPurchasedis saved as a date, not a numbergetDaysBetweenDatesfunction is exported fromutils/dates.jsand imported intoapi/firebase.jsType of Changes
enhancement
Updates
Before
N/A
After
N/A
Testing Steps / QA Criteria
git pullandgit checkout mm-qg-predict-next-purchase.npm start.Listand check one of the items on the list.dateLastPurchased,dateNextPurchasedanddayIntervalshould be updated according to how many days have elapsed since the item was last purchased.