Additional Stores #215

MelissaAutumn · 2023-12-11T21:33:49Z

Resolves #141

I've added some new stores:

CalendarStore -> A cached view of the calendars response. We just pull all of the calendars and then we have some specific getters from retrieving unconnected vs connected calendars. Since calendars are unlikely to change without user action, these can be safely cached until the user does something.
AppointmentStore -> Not as useful, but it's a non-cached view of Appointments. While the appointment endpoint is loading in, it will still provide the potentially out of date data, any new data that shows up should pop in.
ExternalConnectionsStore -> Just a simple store to keep a cached view of the external connections. Like calendars this shouldn't change without user input, so we can just keep this data around until they refresh the page, or do something with it.

These are all per session, so they're not cached in localstorage and will disappear if you refresh the browser. But they'll persist through router navigation.

For what gets access to a store vs a prop: Views should access stores, if a component can potentially take in a generic list of data then it should use a prop, otherwise it can use a store. (Generally components should take in props though.) I'm up for discussion on this though 😄

With #198 this should get rid of any remaining annoying page flashing while requests are happening.

- CalendarStore will only pull if the `isInit` value is false. - CalendarStore cache is busted on user action. - AppointmentStore will always pull if requested (Due to some impl concerns.)

…side the stores.

…ttings page.

MelissaAutumn · 2023-12-11T21:34:40Z

frontend/src/components/SettingsAccount.vue

-const hasZoomAccountConnected = computed(() => (externalConnections.value?.zoom?.length ?? []) > 0);
-const zoomAccountName = computed(() => (externalConnections.value?.zoom[0].name ?? null));
+const hasZoomAccountConnected = computed(() => (externalConnectionsStore.zoom.length) > 0);
+const zoomAccountName = computed(() => (externalConnectionsStore.zoom[0].name ?? null));


Since we only allow one zoom connection, any objection to the zoom getter just grabbing the first item?

No, I think this is fine. Maybe adding a comment indication that we currently only handle one zoom connection would make things clearer here. Is externalConnectionsStore.zoom[0].name ?? null sufficient or do we need another ? before the .name, in case no connection is defined?

frontend/src/views/HomeView.vue

MelissaAutumn · 2023-12-11T21:37:28Z

frontend/src/views/ScheduleView.vue

 // get remote calendar data for current year
 const calendarEvents = ref([]);
 const getRemoteEvents = async (from, to) => {
  calendarEvents.value = [];
-  await Promise.all(props.calendars.map(async (calendar) => {
+  await Promise.all(calendarStore.connectedCalendars.map(async (calendar) => {


We can potentially move this to the calendar store, since schedule probably uses similar code but I'm also good with this living here. Any thoughts?

Yes sounds reasonable to get calendar events over the calendar store 👍🏻

devmount

Again a lot of work improving our code base. Thank you Melissa!

devmount · 2023-12-14T17:01:22Z

frontend/src/App.vue

+import {
+  inject, provide, onMounted, computed,
+} from 'vue';


Do you use a line length limitation < 120? Just wondering about the line breaks here.

I need to fix this up as my ide is at odds with eslint. 😵

devmount · 2023-12-14T17:04:25Z

frontend/src/components/SettingsAccount.vue

-const hasZoomAccountConnected = computed(() => (externalConnections.value?.zoom?.length ?? []) > 0);
-const zoomAccountName = computed(() => (externalConnections.value?.zoom[0].name ?? null));
+const hasZoomAccountConnected = computed(() => (externalConnectionsStore.zoom.length) > 0);
+const zoomAccountName = computed(() => (externalConnectionsStore.zoom[0].name ?? null));


No, I think this is fine. Maybe adding a comment indication that we currently only handle one zoom connection would make things clearer here. Is externalConnectionsStore.zoom[0].name ?? null sufficient or do we need another ? before the .name, in case no connection is defined?

devmount · 2023-12-14T17:06:38Z

frontend/src/stores/appointment-store.js

+        a.calendar_title = calendarsById[a.calendar_id]?.title;
+        a.calendar_color = calendarsById[a.calendar_id]?.color;


At some point we can improve this by appending the corresponding calendar object to the appointment.

frontend/src/views/HomeView.vue

devmount · 2023-12-14T17:09:51Z

frontend/src/views/ScheduleView.vue

 // get remote calendar data for current year
 const calendarEvents = ref([]);
 const getRemoteEvents = async (from, to) => {
  calendarEvents.value = [];
-  await Promise.all(props.calendars.map(async (calendar) => {
+  await Promise.all(calendarStore.connectedCalendars.map(async (calendar) => {


Yes sounds reasonable to get calendar events over the calendar store 👍🏻

…with appointments. We don't need to merge the calendar data anymore since we do on the backend, and we now call the db refresh function on all pages that may require appointments. This prevents doubling requests if you start on the Calendars page for example.

MelissaAutumn · 2023-12-15T18:39:04Z

I have added calendar_title and calendar_color to /me/appointments and removed the calendar part of that merge function. I've also removed the initial call to get calendars and appointments since we already fetch that data where we need it.

This prevents double requests being sent if you start appointments on a page that also request appointment and calendars 😄

MelissaAutumn added 3 commits December 11, 2023 13:02

Add AppointmentStore and CalendarStore.

c4d0333

- CalendarStore will only pull if the `isInit` value is false. - CalendarStore cache is busted on user action. - AppointmentStore will always pull if requested (Due to some impl concerns.)

Hook up external connections store, and move the isLoaded check to in…

5c7d1ab

…side the stores.

We don't need to retrieve appointments or calendars on the account se…

a4d4aa6

…ttings page.

MelissaAutumn requested a review from devmount December 11, 2023 21:33

MelissaAutumn self-assigned this Dec 11, 2023

MelissaAutumn commented Dec 11, 2023

View reviewed changes

frontend/src/views/HomeView.vue Show resolved Hide resolved

MelissaAutumn commented Dec 11, 2023

View reviewed changes

Clean up

8dd4f0f

devmount approved these changes Dec 14, 2023

View reviewed changes

MelissaAutumn added 2 commits December 15, 2023 10:36

Provide calendar title and colour for /me/appointments

b7cb014

MelissaAutumn added 2 commits December 15, 2023 10:41

PR suggestions

b8d0ce6

Merge branch 'main' into features/141-additional-stores

5eae328

MelissaAutumn merged commit 5b905f9 into main Dec 15, 2023
1 check passed

MelissaAutumn deleted the features/141-additional-stores branch December 15, 2023 18:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Additional Stores #215

Additional Stores #215

MelissaAutumn commented Dec 11, 2023

MelissaAutumn Dec 11, 2023

devmount Dec 14, 2023

MelissaAutumn Dec 11, 2023

devmount Dec 14, 2023

devmount left a comment

devmount Dec 14, 2023

MelissaAutumn Dec 15, 2023

devmount Dec 14, 2023

devmount Dec 14, 2023

MelissaAutumn Dec 15, 2023

devmount Dec 14, 2023

MelissaAutumn commented Dec 15, 2023

		a.calendar_title = calendarsById[a.calendar_id]?.title;
		a.calendar_color = calendarsById[a.calendar_id]?.color;

Additional Stores #215

Additional Stores #215

Conversation

MelissaAutumn commented Dec 11, 2023

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

devmount 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

MelissaAutumn commented Dec 15, 2023