feature/df 80 getbudgetbyparams #19

trpubz · 2024-02-10T19:02:23Z

Keypoints

Add query file for BudgetsByParams
Include budgets data into the app js
Include budgetsbyparams in fetch call
Error: export versus import of default
Debug process, changed function input for const to variables for js
Add category selection modal
getBudgetByParams working on budget widet
Get Budget Categories
Remove caching on getBudgetCategories
Float mutation is incorrect for incomes/expenses but connections are live
Fix mutation amount to / 100

Related Issue(s)

closes DF-80

Checklist:

New/Modified Tests are Passing
I have commented my code, particularly in hard-to-understand areas.
I have made corresponding changes to the documentation.
My changes generate no new warnings or errors.

Additional Notes:

Please enusre you pull to local, test localhost rails server and live server

…live

linear · 2024-02-10T19:02:26Z

DF-80 getBudgetByParams

vercel · 2024-02-10T19:02:29Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
dough-fin-fe	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 10, 2024 9:10pm

Little adjustments for titleizing the display on some hard-coded text.

chisPmama · 2024-02-11T18:40:33Z

src/components/AddIncome/AddIncome.js

@@ -51,7 +51,7 @@ const AddIncome = ({ totalIncome, setTotalIncome, setTransactions }) => {
        id: data.createIncome.id,
        vendor: source,
        date,
-        amount: amountCents,
+        amount: amountCents / 100,


This has an issue with being displayed on Transactions as $5.00 if the user actually wrote $500.00.

Could you look into this?

I'm not able to address this today...whoever coded the original logic might be able to lend some advice but there is certainly a disconnect between prop passing and the mutation/queries. This referenced line of code ensures the proper value is sent back through the mutation. Not sure if the issue is closer to data manipulation on the query (for transactions) or just on how it's displayed.

Here's my thought. @trpubz your branch still does not appear have the correct useCreateIncome or useCreateExpense mutation that is on main and main seems to be functioning correctly updating state just not posting since the mutation merge yesterday.

Here's how the FE app is handling floats for example with totalIncome:
The getIncomes hook fetches and returns the income as a float

Then to avoid rounding errors, the useEffect handling setting income state coverts the float into a cents integer:

Then whenever any amount needs to be displayed in the App (income, expenses, transaction etc.) it's formatted into a USD currency string:

This means we need to adhere to how floats are handled on main.
A few more thoughts:

Pretty sure this is a bug, it's a matter of this branch not having parts of main like the useMutation fixes from Friday.

We are allowed to fix bugs, but this is a feature branch.

Lets leave this alone, I'm at a loss of words to be honest at this point..

chisPmama

Just one hopefully minor change. The AddIncome displays incorrectly in transactions.

MaryBruff · 2024-02-11T15:31:19Z

cypress/e2e/Dashboard.cy.js

This file seems to just contain the tests @josephlee702 wrote, any particular reason they've been copied from his branch and not left on the Cypress branch?
If you're going to add Cypress tests, make sure they test the component you built.

MaryBruff · 2024-02-11T15:31:31Z

src/components/App/App.js

  const userName = "Powdered Toast Man";
  const email = "[email protected]"
-
+  localStorage.setItem('email', email);


This line can be removed since the localStorage object is not being retrieved anywhere and email is set in the line above.

MaryBruff · 2024-02-11T15:31:35Z

src/components/App/App.js

  const { totalIncomeData } = useGetIncomes(email);
  const { totalExpensesData } = useGetExpenses(email);
  const { transactionsData } = useGetTransactions(email);
  const { cashFlowData } = useGetCashFlow(email);
+  // const { budgetsData } = useGetBudgetsByParams(month, category, email);


Like all other useQuery hooks that retrieve data when the App mounts, the useGetBudgetByParams should follow the DDAU design convention.

MaryBruff · 2024-02-11T15:31:39Z

src/components/Budget/BasicPie.js

-export default function PieActiveArc() {
+export default function BasicPie({ data }) {
+  // Validate incoming data - simple example
+  const isValidData = data && Array.isArray(data) && data.length > 0 && data.every(d => d.hasOwnProperty('value') && typeof d.value === 'number');


When the budgetData is properly drilled to this component, the chained conditionals can likely be reduced to something similar in the BarChart MUI component.

MaryBruff · 2024-02-11T15:31:41Z

src/components/Budget/Budget.js

@@ -1,48 +1,148 @@
-import React from 'react'
+import React, { useState, useRef, useEffect, useMemo } from 'react'


Remove unused hook.

MaryBruff · 2024-02-11T15:31:43Z

src/components/Budget/Budget.js


 const Budget = () => {
-  return (
+    const email = "[email protected]";


This entire blocked dedicated to fetching budgets and budgetCategories should be be properly drilled through from App. Those states should not be set inside this child component.

MaryBruff · 2024-02-11T15:31:44Z

src/components/Budget/Budget.js

+        setIsDropdownVisible(false); // Hide the modal
+    };
+    // Handler to toggle dropdown visibility
+    const toggleDropdownVisibility = () => {


Consider refactoring this to utilize a BasicSelect MUI component, there is no need to make these kinds of functions.

MaryBruff · 2024-02-11T15:31:47Z

src/components/Budget/Budget.js

+        document.addEventListener('mousedown', handleClickOutside);
+        return () => {
+            document.removeEventListener('mousedown', handleClickOutside);
+        };


The BasicSelect component will remove the need for EventListeners like this. VanillaJS in React is not maintainable/conventional

MaryBruff · 2024-02-11T15:31:56Z

src/fetchCalls.js

No need to add anything new to this file, it's deprecated.

MaryBruff · 2024-02-11T19:55:31Z

src/components/AddIncome/AddIncome.js

@@ -51,7 +51,7 @@ const AddIncome = ({ totalIncome, setTotalIncome, setTransactions }) => {
        id: data.createIncome.id,
        vendor: source,
        date,
-        amount: amountCents,
+        amount: amountCents / 100,


Here's my thought. @trpubz your branch still does not appear have the correct useCreateIncome or useCreateExpense mutation that is on main and main seems to be functioning correctly updating state just not posting since the mutation merge yesterday.

Here's how the FE app is handling floats for example with totalIncome:
The getIncomes hook fetches and returns the income as a float

Then to avoid rounding errors, the useEffect handling setting income state coverts the float into a cents integer:

Then whenever any amount needs to be displayed in the App (income, expenses, transaction etc.) it's formatted into a USD currency string:

This means we need to adhere to how floats are handled on main.
A few more thoughts:

Pretty sure this is a bug, it's a matter of this branch not having parts of main like the useMutation fixes from Friday.

We are allowed to fix bugs, but this is a feature branch.

Lets leave this alone, I'm at a loss of words to be honest at this point..

josephlee702 · 2024-02-23T16:12:47Z

Was part of capstone pt.1 - closing.

chisPmama and others added 12 commits February 9, 2024 11:15

Add query file for BudgetsByParams

0baa781

Include budgets data into the app js

8e7c735

Include budgetsbyparams in fetch call

f464056

Error: export versus import of default

3807694

Debug process, changed function input for const to variables for js

979ad4d

Add category selection modal

e1536c0

getBudgetByParams working on budget widet

4f33361

Get Budget Categories

4516a09

Merge fixes from bug/mutations branch

72e4ce2

Remove caching on getBudgetCategories

cbfcd34

Float mutation is incorrect for incomes/expenses but connections are …

4214f34

…live

Fix mutation amount to / 100

03e9ae1

trpubz requested review from ben-rosner-williamsburg, chisPmama, josephlee702, MaryBruff and shawndcarpenter February 10, 2024 19:02

Update Budget.js

4df1494

Little adjustments for titleizing the display on some hard-coded text.

chisPmama approved these changes Feb 10, 2024

View reviewed changes

Merge branch 'main' into feature/df-80-getbudgetbyparams

df95a16

vercel bot deployed to Preview February 10, 2024 19:11 View deployment

trpubz marked this pull request as draft February 10, 2024 19:20

Change userId to 2

e85c626

vercel bot deployed to Preview February 10, 2024 21:03 View deployment

trpubz marked this pull request as ready for review February 10, 2024 21:04

Added cypress tests

88c0a3c

vercel bot deployed to Preview February 10, 2024 21:10 View deployment

chisPmama reviewed Feb 11, 2024

View reviewed changes

chisPmama requested changes Feb 11, 2024

View reviewed changes

MaryBruff requested changes Feb 12, 2024

View reviewed changes

josephlee702 closed this Feb 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature/df 80 getbudgetbyparams #19

feature/df 80 getbudgetbyparams #19

trpubz commented Feb 10, 2024

linear bot commented Feb 10, 2024

vercel bot commented Feb 10, 2024 •

edited

Loading

chisPmama Feb 11, 2024

trpubz Feb 11, 2024

MaryBruff Feb 11, 2024

chisPmama left a comment

MaryBruff Feb 11, 2024

MaryBruff Feb 11, 2024

MaryBruff Feb 11, 2024

MaryBruff Feb 11, 2024

MaryBruff Feb 11, 2024

MaryBruff Feb 11, 2024

MaryBruff Feb 11, 2024

MaryBruff Feb 11, 2024

MaryBruff Feb 11, 2024

MaryBruff Feb 11, 2024

josephlee702 commented Feb 23, 2024

		@@ -1,48 +1,148 @@
		import React from 'react'
		import React, { useState, useRef, useEffect, useMemo } from 'react'

feature/df 80 getbudgetbyparams #19

feature/df 80 getbudgetbyparams #19

Conversation

trpubz commented Feb 10, 2024

Keypoints

Related Issue(s)

Checklist:

Additional Notes:

linear bot commented Feb 10, 2024

vercel bot commented Feb 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chisPmama left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josephlee702 commented Feb 23, 2024

vercel bot commented Feb 10, 2024 •

edited

Loading