search

rubyforgood / pantry_scheduler

MIT License
7 stars 13 forks source link

Family size on dashboard summary doesn't match total of adults and children #72

Closed pollygee closed 7 years ago

pollygee commented 7 years ago

If family size is updated in the appointment, then Client needs to be updated.

pollygee commented 7 years ago

Family size should always be fetched from Client. What displays on the dashboard should be what's stored in Client not what's stored in Appointment.

jgaskins commented 7 years ago

What's the reasoning behind that? If they made an appointment for a family size of 8, it seems to make more sense to go off of what the appointment says, right? If we go by what's on Client, the Appointment's value becomes unnecessary. In a real-world scenario, though, it seems like they should match up on the dashboard anyway.

We denormalized that data in the case of changing family sizes for clients so we know how much food was given each month for reports regardless of the client's current family size. If it doesn't match the appointment's size, the report becomes inaccurate.

pollygee commented 7 years ago

I guess what we should say here is that if the family size gets edited in the appointment then Client needs to be updated.

It doesn't make any sense to be looking at an appointment and the family size is different than what it is on the client. So if updating at the time of the appointment we need to up date the client so it's changed, matches and will be ready for the next appointment.

This will help with the reports as well.

jgaskins commented 7 years ago

I guess what we should say here is that if the family size gets edited in the appointment then Client needs to be updated.

I love this idea.

ajohnson052 commented 7 years ago

On a related note: If you save family size on the checkin modal, it will update the client and not the appointment.

Maybe we could address this by nesting a hidden field for the appointment in the form. Since appointments should always update their clients, it seems smart to take care of that on the backend. However, I don't think that we want to assume a client update always updates the most recent appointment.

pollygee commented 7 years ago

You're right, when editing the family size on the checking modal it will need to update the Client and the Appointment. I also agree that appointments should always update their Clients.

Client update should only update appointments that haven't happened yet. If it's in the past then the appointment should stay because that was the family size then, and future appointments will be for the new updated family size or whatever.

ajohnson052 commented 7 years ago

Good point! We can still do this work on the backend if it applies to all future appointments rather than checking for the last appointment made.

ajohnson052 commented 7 years ago

Has production data already been populated for this? It would be helpful to store num_adults and num_children on appointments in order to make the appointment update it's client automatically. An easy fix if the app hasn't been used in production yet. More tricky if it's already being used.

pollygee commented 7 years ago

OK so you think this is the case for both senarios? In which case maybe we should create another ticket as this is a different solution? I think it sounds good though.

So something like.... if Appointment changes, Client needs to get updated. If Client changes all future appointments need to get updated?

pollygee commented 7 years ago

Production data has not been updated yet. We have quite a few critical pieces to complete before this is a MVP.

num_adults and num_children is stored on Appointment, but the modal only shows the total of family size. This should be changed actually because when creating the appointment the operator will ask the client the makeup of their family and if it changes they would change it in the process of creating the appointment (which should then update the client).

ajohnson052 commented 7 years ago

That's good news!

num_children and num_adults are currently only on Client. But adding them to Appointment is a simple change if we don't need to worry about production data.

I think I have enough information to tackle this bug! Thanks for the help!

pollygee commented 7 years ago

Oh yes! You're right, that is certainly a problem.....I don't think we were thinking about needing to update the client when creating the appointment but this is def needed for the workflow of the volunteers. Thanks for thinking this through with me!!