Site Data Refactor to make similar to CustomerData and InventoryData #83
Conversation
There was a problem hiding this comment.
I'm a little confused about the new structure with full TariffPlan objects under individual sites since AFAIK a tariff plan can still be shared across multiple sites right? Wouldn't this lead to conflicts with editing tariff plans? I thought we landed on the original plan with sites only tracking tariff plan ids and referencing a shared list of tariff plans that we keep as the single source of truth.
Besides that, I left some nitpicks + renaming suggestions. Everything else looks good!
src/lib/redux/siteData.ts
Outdated
| (siteId, state) => state.siteData.sites[siteId].siteInformation) | ||
|
|
||
| // Returns all SiteRecord[] information | ||
| export const selectAllSites = (state: RootState) : SiteRecord[] => Object.values(state.siteData.sites).map(site => site.siteInformation); |
There was a problem hiding this comment.
Also rename to selectAllSitesInformation?
Also, why did you access state.siteData.sites directly? It seems like our pattern so far has been to use the entity adapter's getSelectors and build custom selectors off of those, so I'm curious why you did this instead
There was a problem hiding this comment.
Will rename to that. I originally had it at that name, but thought I might be breaking convention since others are like selectAllCustomers etc
I didn't know a better way to do it. state.siteData.sites.siteInformation (only tariffPlans and financialSummaries) isn't an adapter, so there aren't pre-given selectors for it.
src/lib/utils/customerUtils.ts
Outdated
| export const getTariffPlan = (customer: CustomerRecord, currentSite: SiteRecord): TariffPlanRecord | undefined => { | ||
| const tariffPlanId = customer.tariffPlanId; | ||
| return currentSite.tariffPlans.find((plan: TariffPlanRecord) => plan.id === tariffPlanId); | ||
| export const getTariffPlan = (customer: CustomerRecord): TariffPlanRecord | undefined => { |
There was a problem hiding this comment.
Rename to something like getTariffPlanByCustomer? You could also just turn this into a selector similar to selectProductByInventoryId.
There was a problem hiding this comment.
That's a better name, thanks. Will do getTariffPlanByCustomer first. Might be good to change it to a selector once the CustomerProfile PR is finished (don't wanna cause more conflicts than necessary)
… julian/site-data-refactor
Discussed on slack. Confirmed with NPO that tariff plans are bound to only a single site. We both agreed that adapters are a good solution with this constraint |
What's new in this PR
Customer Data and Inventory Data have a nice redux structure that allows for selector use and entity adapters. This PR refactors siteData so that it has the same structure. This was done now because we will begin implement functionality around objects that live in
SiteDatasuch as Financial Summaries and Tariff Plans.Changelog
siteDatastructure identical toinventoryDataandcustomerDatasiteDatanow has 2 entity adapters per site: 1 for Financial Summaries and 1 for tariff planscurrentSiteand switched them to use the selectorselectCurrentSiteHow to review
currentSiteare still reachableRelevant Links
Online sources
Related PRs
Next steps
Screenshots
Evidence of no more
currentSiteCC: @julianrkung