https://frum.finance logo
Join Slack
Powered by
<@U08QX3AF5RD> let’s create a thread on this messa...
# software-contributors
f
@ripe-lifeguard-67276 let’s create a thread on this message an knock out all the open frontend points
m
Let's do it!
r
Here!
To summarize: 1. Household Profile - its commented out, but not deleted. I'll fill with sample data as requested. 2. The original BudgetForm with options for subcategories is all still there, but I renamed the file "BudgetFormExpanded" 3. BudgetAnalysis was renamed to Insights, just so that the names were more consistent across the codebase. But I hardly changed actual code over there. 4. When inputting gender for children in the demographics, right now there's no gender field on the backend, so that's commented out, but not deleted. 5. The original BudgetPage which controls the flow between the BudgetForm and the BudgetAnalysis is also still there, just commented out 6. Everything with onboarding/login is still there, except at the moment there are no routes for it. Users can't access it.
m
i unresolved a few threads also
1. Household Profile - its commented out, but not deleted. I'll fill with sample data as requested. 👍 Thanks, no reason to comment out when we can still use it 1. The original BudgetForm with options for subcategories is all still there, but I renamed the file "BudgetFormExpanded" I think the code will quickly become stale, better to hide it behind a flag since hopefully it will be implemented very soon and we need it. 1. BudgetAnalysis was renamed to Insights, just so that the names were more consistent across the codebase. But I hardly changed actual code over there. 👍 1. When inputting gender for children in the demographics, right now there's no gender field on the backend, so that's commented out, but not deleted. 👍 1. The original BudgetPage which controls the flow between the BudgetForm and the BudgetAnalysis is also still there, just commented out I think we still want to keep that in, and it should handle both entry points 1. Everything with onboarding/login is still there, except at the moment there are no routes for it. Users can't access it. I think having a route could be helpful as long as we make it an unlikely route someone would find, so we can keep testing it to make sure things work
@full-wall-37598 @ripe-lifeguard-67276 Thoughts on this from the AI? • 
src/api/budget.tsx:26-35
- Catching 500 errors and treating them as "no data" is problematic • This masks actual server errors and makes debugging difficult • Should distinguish between "no data found" (404) and "server error" (500)
r
1. I think we still want to keep that in, and it should handle both entry points ReportPage (which is whats being rendered now) is basically just a refactored version of BudgetPage. Nothing fundamental was really changed. It handles both entry points and just adds in the option to input demographics if needed. It also synchronizes data flow to and from the database.
m
right so we shouldn't need to comment anything out i should stil be able to get to that page through a different route that allows be to onboard and get to that page like before
this should be additional functionality, not replacing the exisitng
r
okay so I'll add back in the onboarding flow. after the user onboards however, rendering ReportPage should be good from there, no? Since it does the same thing as BudgetPage. Also, BudgetPage doesn't get any data from our backend, just default data.
m
which flow are we talking about ? web login or slack login?
web login would go to the place to manually put in budget entries, which currently doesn't work yet
or at least i don't think it does
maybe it does
r
oh i think i understand where I'm missing you now. as its structured now: ReportPage handles both options -> whether a user gets to the ReportPage from a slack login or the onboarding flow, they will still 1. be able to see their budgets and 2. after submission, see their insights. the difference is 1. if the user came from slack (and therefore connected their ynab) then the input fields on the budgets will be disabled. Basically the user can just review them to make sure they are correct and then "submit". 2. if the user came from the web onboarding, then the input fields will be enabled.
f
@ripe-lifeguard-67276 I just pushed a pr that makes external functional
so we can go off that field
r
fantastic, its implemented in the frontend
At least this is my understanding, is this what was intended?
m
maybe i am misunderstanding what the report page has on it, but i thought the report page is the analysis so there wouldn't be a check and submit feature. That would already be from the budget page for a web user or the slack page for a ynab user
f
@ripe-lifeguard-67276 slack users might not be ynab connected so external will be false and they will manually enter
🎯 1
m
So slack should go to the budget entry page, not the report page
the report page should be agnostic to the user type
f
@mysterious-tomato-10057 it’s dynamic - if the budgets are not yet final then they can review it, if it is final they go straight to analysis
m
Awesome
r
@full-wall-37598 true, im disabling the input fields based off of "external"
✅ 1
m
i think the code and comments could mkae that clearer
👍 1
i don't know what "external" means for example
f
that the budget data is from an external source (i.e. ynab) to allow for future integrations perhaps
m
hmm, gotcha, needs to be named better IMO
is_budget_externally_sourced or whatever for clarity
f
question for both of you - @ripe-lifeguard-67276 if a user budget doesn’t come through, in theory the user can mix ynab and manual budgets?
so they have an external budget “food” - it’s disabled can they manually enter transportation?
m
i vote no.
r
I was actually going to ask about that!
In the code now, its not supported
m
all or nothing otherwise too confusing
f
so @ripe-lifeguard-67276 we’re good?
r
yeah
m
❤️
r
@full-wall-37598 It does however make the code a little ugly, just because I'm checking one category (i.e budgets[0].external) to decide for all of them
✅ 1
f
we can move to top level at some point
r
okay, as long as that check is good for now.
f
it is
m
hmm, is the backend guaranteeing they will all be the same?
f
yes
m
kk
r
@mysterious-tomato-10057 so is it okay that ReportPage dynamically handles both slack users and web users?
m
what are the two different behaviors of the report page? i'd think its agnostic
r
Since the flow of a user from slack vs a user that onboarded directly is basically identical: 1. see budgets 2. see analysis I thought they could be combined into one flow. The only difference is whether the inputs are disabled or not. So budgets from "external" sources will be view-only, and users with no budgets yet will be able to input their spending
m
the budget page should have the budget and the analysis page should have the anlaysis
i'm not sure i'm following
the budget page should be dyanmic based on external source, but the analysis page would be agnostic
r
sorry, not sure i understand what you mean by agnostic
the budget analysis page is the same for everyone
and the budget form page is based off of external source
m
right
r
ReportPage is in charge of fetching the data and controlling the flow, deciding when to show the Budget Form or the Budget Analysis
f
agnostic meaning can be used for either one in the same way
which we do because you just care if there’s a session or not
r
oh yeah its agnostic
m
oh i think that's my issue then, the report page should just be showing a report, based on the naming, it shouldn't be the one responsible for showing me a budget form
r
ohhh
i can rename it to whatever makes more sense
m
if its the whole app i would maybe call it budge insights
but idk what the overall naming is i just don't think a report should ask me for input
f
it will just show you different components base on what’s needed (i.e. demographics, budgets manual entry)
if they do the standalone onboarding when it’s ready - they will never see demographics there
m
that sounds like the whole budget insights ap then
app
f
most of it ye
m
so should be named accordingly
r
agree - I'll rename it
f
there are a few places they can view that same component that gathers information - so we’re not redirecting them to another page but rather in a few different pages showing the same component
meaning demographics input can be shown in onboarding or here
m
right that's fine, just name it accordingly then
f
or in theory in their settings page if you wanted to allow that
(which ik you don’t)
r
So I think we officially don't need the original BudgetPage? Since its functionality is encapsulated and extended by the now "BudgetInsightsPage"
m
i'm not sure what page is what
r
BudgetInsightsPage is what I renamed the ReportPage. Basically the whole budget insights app
Side point: i apologize for lumping all of these changes into one pr! It makes it really confusing to understand
m
no worries! it might be easier to split it out even retroactively for us to review this quicker,
f
at this point I vote let’s power through and learn from it 🔥
💪 1
r
😄
💪 1
So the original BudgetPage shows a form to input spending, and on submission (which is passed through state), then renders an analysis (which has no data because nothing is being fetched from the backend)
the BudgetInsightsPage shows a form to view/edit spending based off of api data. Then on submission saves to a database. Then shows the analysis which is fetched from the backend.
the only reason its a separate file is just because I found it easier to neatly integrate all the api calls by copy pasting the original code and working from there
m
so the code was forked and you edited it and now you're asking if you can delete the original?
r
basically, because the originals functionality is all in the new one
just integrated with the api
m
i mean, if its not missing any features then yeah it sounds like duplicate code that wouldn't make sense to keep
r
yeah its not missing any features. the features related to subcategories are in different components and I'll put those behind a flag.
m
kk so then it sounds like there is no question as long as no code or features are being deleted
👍 1
Open in Slack
PreviousNext
Powered by