grassrootsgrocery / admin-portal

GNU Affero General Public License v3.0
10 stars 5 forks source link

88 inline participant editing #106

Closed 6mp closed 1 year ago

railway-app[bot] commented 1 year ago

This PR is being deployed to Railway đźš…

admin-portal: ◻️ REMOVED

mattsahn commented 1 year ago

@6mp, can you add some detail to the PR description on what you've implemented in this PR? Also, include screenshots to show before/after.

6mp commented 1 year ago

Sorry about that I made the PR through my IDE and I guess none of the other stuff got pushed with it.

This PR allows inline editing of volunteers for an event, the fields that can be edited are:

Before:

image

After:

image

Edit Box:

image
danthefridgeman commented 1 year ago

That would be helpful to have but only with the expanded user permission

Also important to be able to change someone from a sorter only to a driver for example or to change someone’s event from one Saturday to another

On Sun, Jul 23, 2023 at 7:13 PM Matt Sahn @.***> wrote:

@.**** commented on this pull request.

In server/routes/volunteers.ts https://github.com/grassrootsgrocery/admin-portal/pull/106#discussion_r1271575152 :

  • }: {
  • firstName: string;
  • lastName: string;
  • email: string;
  • phoneNumber: string;
  • participantType: string[];
  • } = req.body;
  • const stringFields = [firstName, lastName, email, phoneNumber];
  • const isValidRequest = stringFields.every((field) => {
  • return field.trim().length > 0;
  • });
  • // also verify that the participantType is valid
  • const validParticipantTypes = ["Driver", "Packer"];

Overall, this is a really impressive PR, @6mp https://github.com/6mp! Would be great if some others could review the code @.*** https://github.com/jasoncavanaugh, @zuechai https://github.com/zuechai ). Also, @danthefridgeman https://github.com/danthefridgeman, just checking on the functionality here you want. With this little popup, the admin will be able to edit the person's actual name, phone number, and email in the master CRM table. That's pretty cool, but also a lot of power so want to make sure that's desired in this spot.

— Reply to this email directly, view it on GitHub https://github.com/grassrootsgrocery/admin-portal/pull/106#discussion_r1271575152, or unsubscribe https://github.com/notifications/unsubscribe-auth/AXAWI5ZEYMOJW7AVJRZONK3XRWVZZANCNFSM6AAAAAA2NAX46E . You are receiving this because you were mentioned.Message ID: @.***>

--

Dan Zauderer Founder, Grassroots Grocery 917-497-2514

Check us out on Kelly Clarkson http://www.grassrootsgrocery.org/grassroots-on-kelly! Make a gift here http://www.grassrootsgrocery.org/donate!

Note, I honor and respect boundaries around personal time, self-care, caregiving, and rest. If you receive correspondence from me during a time when you’re engaging in any of the above, please protect your energy and respond when you have the capacity.

danthefridgeman commented 1 year ago

What I mean is that I’d like to be able to change someone from just a driver to a driver and a packer or just a packer to a packer and a driver … but that would also require changing time slots in some cases and such

On Sun, Jul 23, 2023 at 7:20 PM matt @.***> wrote:

@.**** commented on this pull request.

In server/routes/volunteers.ts https://github.com/grassrootsgrocery/admin-portal/pull/106#discussion_r1271576103 :

  • }: {
  • firstName: string;
  • lastName: string;
  • email: string;
  • phoneNumber: string;
  • participantType: string[];
  • } = req.body;
  • const stringFields = [firstName, lastName, email, phoneNumber];
  • const isValidRequest = stringFields.every((field) => {
  • return field.trim().length > 0;
  • });
  • // also verify that the participantType is valid
  • const validParticipantTypes = ["Driver", "Packer"];

Doesn't this need to include "Driver & Packer", too?

If a volunteer is both a driver and packet I send and array with ["Driver", "Packer"] since thats the format airtable takes it in. If they're one or the other it's just a single array like ["Driver"].

As for permissions I will update this endpoint to only be used by admins afterr I make my pr for issue #52 https://github.com/grassrootsgrocery/admin-portal/issues/52 which will restrict functionally of the admin portal to specified users.

— Reply to this email directly, view it on GitHub https://github.com/grassrootsgrocery/admin-portal/pull/106#discussion_r1271576103, or unsubscribe https://github.com/notifications/unsubscribe-auth/AXAWI52OWSN3LG3MLNFXEMTXRWWUBANCNFSM6AAAAAA2NAX46E . You are receiving this because you were mentioned.Message ID: @.***>

--

Dan Zauderer Founder, Grassroots Grocery 917-497-2514

Check us out on Kelly Clarkson http://www.grassrootsgrocery.org/grassroots-on-kelly! Make a gift here http://www.grassrootsgrocery.org/donate!

Note, I honor and respect boundaries around personal time, self-care, caregiving, and rest. If you receive correspondence from me during a time when you’re engaging in any of the above, please protect your energy and respond when you have the capacity.

6mp commented 1 year ago

@danthefridgeman Currently this allows you to change someones volunteer type from "driver" to "driver and packer" or "driver and packer" to just "packer" but it doesn't account for the time slots.

danthefridgeman commented 1 year ago

Yeah so it wouldn’t actually work because they’d also need a slot added.

That could be a back end airtable automation for when packer is added to a driver that the driving slot changes to 10:30am and that the 9am packing slot gets added -

And when driver is added to packer that the 10:30am driving slot automatically gets added

Could happen through airtable automations I believe

Things happen through time slots for both roles and so those would have to be taken into account in some way — pretty sure it could be automated except for the fact that we also might be running out of airtable automations possible in a single base?

Does someone want to text me @ 917-497-2514 to get more clarity on what I mean here

Thanks so much everyone for the hard work on this - can’t wait for it to be more functional.

On Sun, Jul 23, 2023 at 7:46 PM matt @.***> wrote:

@danthefridgeman https://github.com/danthefridgeman Currently this allows you to change someones volunteer type from "driver" to "driver and packer" or "driver and packer" to just "packer" but it doesn't account for the time slots.

— Reply to this email directly, view it on GitHub https://github.com/grassrootsgrocery/admin-portal/pull/106#issuecomment-1646992521, or unsubscribe https://github.com/notifications/unsubscribe-auth/AXAWI52VWHGFUEEHE6Y5HOLXRWZUDANCNFSM6AAAAAA2NAX46E . You are receiving this because you were mentioned.Message ID: @.***>

--

Dan Zauderer Founder, Grassroots Grocery 917-497-2514

Check us out on Kelly Clarkson http://www.grassrootsgrocery.org/grassroots-on-kelly! Make a gift here http://www.grassrootsgrocery.org/donate!

Note, I honor and respect boundaries around personal time, self-care, caregiving, and rest. If you receive correspondence from me during a time when you’re engaging in any of the above, please protect your energy and respond when you have the capacity.

jasoncavanaugh commented 1 year ago

So I tried to run this locally against the dev airtable base, and I got an unrelated error in a different part of the app. I think it has to do with the fact that a field got added to the prod airtable base but not the dev one, which made the /api/events call fail. Is it possible that we try to keep the dev base up to date with prod? For something like this, you need to edit the data in order to test out the feature, so it seems better to be able to test that against the dev base instead of messing with the actual data in prod.

6mp commented 1 year ago

@jasoncavanaugh What is the error you are getting? I am also running it locally against the dev airtable (Demo base w/o personal data) and don't have any errors with the /api/events call.

mattsahn commented 1 year ago

So I tried to run this locally against the dev airtable base, and I got an unrelated error in a different part of the app. I think it has to do with the fact that a field got added to the prod airtable base but not the dev one, which made the /api/events call fail. Is it possible that we try to keep the dev base up to date with prod? For something like this, you need to edit the data in order to test out the feature, so it seems better to be able to test that against the dev base instead of messing with the actual data in prod.

yes, good point. Let us know the exact error/place and which base you're coding against.

jasoncavanaugh commented 1 year ago

I'm seeing

Airtable response: {"error":{"type":"UNKNOWN_FIELD_NAME","message":"Unknown field name: \"Only Distributor Count Including Unconfirmed\""}}

When I go to the dev airtable base, I don't see this field on Supplier Pickup Events. image

6mp commented 1 year ago

@jasoncavanaugh Are you using Demo Base w/o Personal Data when testing locally? That and prod are the only 2 databases with the updated fields.

mattsahn commented 1 year ago

@jasoncavanaugh Are you using Demo Base w/o Personal Data when testing locally? That and prod are the only 2 databases with the updated fields.

@6mp, can you also update "GitHub Development Base" at https://airtable.com/appSd2vxc7Pxepa7t/tblVFZ1bmAfXlpVSc/viwblbaKkJ8UVyZx9?blocks=hide? That's actually been the base we've been using for new devs, but we need to do some cleanup of the bases overall and probably have people re-clone from that one periodically.

@jasoncavanaugh, let us know the URL or name of the base you're using.

mattsahn commented 1 year ago

Yeah so it wouldn’t actually work because they’d also need a slot added. That could be a back end airtable automation for when packer is added to a driver that the driving slot changes to 10:30am and that the 9am packing slot gets added - And when driver is added to packer that the 10:30am driving slot automatically gets added Could happen through airtable automations I believe Things happen through time slots for both roles and so those would have to be taken into account in some way — pretty sure it could be automated except for the fact that we also might be running out of airtable automations possible in a single base?

Getting back to the issues with changing type, @danthefridgeman is right that there's some complexity that needs to be handled with the slots. But, I also think it's all doable from the backend code here and no need to push to Airtable automations. The scenarios are:

  1. Driver -> Driver & Packer - The volunteer has a single "Scheduled Slots" record for the date in question. That record links to a record in the "Available Slots" table for the same date and with a specific time via the "Driving Slot" link/field. If this driver is linked to anything other than the 10:30am Driving Slot for that day, it must be changed to instead link to the 10:30am record, since, as a Packer, the driver will be available to drive after the packing finishes at 10:30am. So, if they were originally scheduled for 11:30am driving slot, it needs to change to 10:30am.

Then, the same single "Scheduled Slots" record for the volunteer must be linked, via the "Logistics Slot" link/field, to the "Available Slot" record for that date that is for 9am Distributor. That's a single record, but will have many "Scheduled Slots" records linked to it, one for each person signed up as a Distributor/Packer that date.

So, after the above 2 operations, the Driver will now have two "Available Slots" linked to from their "Scheduled Slots" record, one as a 9am Distributor and one as a 10:30am Driver.

  1. Packer -> Driver & Packer - This is similar to above, but probably a little simpler. As a Packer, they will have a Scheduled Slot record for 9am Distributor which doesn't need to change at all. They just need to have link added to the 10:30am Driver slot for that date.

Probably best to start with case #2 as the easier one first. I think this should all be possible in the backend, but it's definitely tricky to be doing updates to linked records. @hgreenhut, what do you think about doing this in the backend code here vs putting logic in Airtable?

jasoncavanaugh commented 1 year ago

Ok, I did not know we had another dev base other than the one I was using. I will switch to the one without personal data. I believe the one I was using was called Grassroots (DEV) but I will have to double check on my personal computer when I get the chance.

henrygreenhut commented 1 year ago

We definitely could handle adjusting driving & logistics slots automatically through an Airtable script -- but I could see it getting in the way if every time you adjusted someone's volunteering type in Airtable, their slot times kept changing as well, particularly when it comes to debugging or just edge cases where they might have a different slot time. I think it's best to keep that logic to the portal backend, both for simplicity and also to keep Airtable flexible.

6mp commented 1 year ago

I believe that this is completed, I have tested the following on my development airtable base:

Packer only -> Driver only Packer only -> Driver & Packer

Driver only -> Packer only Driver only -> Driver & Packer

Driver & Packer -> Driver only Driver & Packer -> Packer only

I tested these with a normal volunteer and a special event volunteer and everything looked good.

danthefridgeman commented 1 year ago

Amazing - does that mean that we can merge it into the main dashboard just to check it out?!

-Dan

Dan Zauderer Founder, Grassroots Grocery 917-497-2514

Check us out on Kelly Clarkson https://t.sidekickopen69.com/Ctc/OR+23284/d2nMfk04/JkM2-6qcW6N1vHY6lZ3p6W9bw7Hy3LBl-4W2-Db7m1rqN05W6CsxD67svq_bW5YQmSr6jl0jGV8CMKw6sFMWKW2k8NhH2rmkjmW8RCWpM2V3j87W6ZNn9W2vn-9YW7cgSPy1X8qHlW8N2flh6FDLb_N1bzrPcfqkk4W7R4qlq5mwyVvW5bmg4-52t5KbW1Yg09L1frbN9W6l5Pnb8nXFFlW2DVl0k3H8gyzW2zB4Vj5Gc9w5W5VJgDg7LN68TW5-7QxB6sV3ZYW3cmBHx5zbxtgW4T-vmz7-Sg8_W3rGt7c8RhT3Vf4pYGqW04! Make a gift here https://t.sidekickopen69.com/Ctc/OR+23284/d2nMfk04/Jks2-6qcW69sMD-6lZ3krW2GNLpg15gq72W2FK5kc8bVrGpW1vhx0Z49r6XJW8Xg46k42G68KN8ds4S0c01p2W30BX823h3128N63gX5Q-vZPJW5lLL676NJhN6W2QGR3C5Rk0s9W4ghCP791DRzjW3ZGF216hlwYxW7BLMcj99dcRdW8FrZyz8p_vT8W7_xjGD4HqwrlW93BNvM9fT_h2N6jZ00sgkBpDW7DdgcL4RKx5HW89sxBL23GrZLW1SMhjS2dVtM0N3SzrPKCXSXFd-KBs404 !

Note, I honor and respect boundaries around personal time, self-care, caregiving, and rest. If you receive correspondence from me during a time when you’re engaging in any of the above, please protect your energy and respond when you have the capacity.

On Wed, Aug 16, 2023 at 1:24 PM matt @.***> wrote:

I believe that this is completed, I have tested the following on my development airtable base:

Packer only -> Driver only Packer only -> Driver & Packer

Driver only -> Packer only Driver only -> Driver & Packer

Driver & Packer -> Driver only Driver & Packer -> Packer only

I tested these with a normal volunteer and a special event volunteer and everything looked good.

— Reply to this email directly, view it on GitHub https://github.com/grassrootsgrocery/admin-portal/pull/106#issuecomment-1681004918, or unsubscribe https://github.com/notifications/unsubscribe-auth/AXAWI56RLVR37ZZVPFTVWH3XVT627ANCNFSM6AAAAAA2NAX46E . You are receiving this because you were mentioned.Message ID: @.***>

6mp commented 1 year ago

@danthefridgeman To test on the live airtable you can use this link: https://admin-portal-admin-portal-pr-106.up.railway.app/

Its the same login as the normal website

danthefridgeman commented 1 year ago

Wow, so cool! The only thing that's now off is this delivery and location info page where I am getting the spinning wheel of death [image: Screen Shot 2023-08-16 at 2.52.47 PM.png]

Dan Zauderer Founder, Grassroots Grocery 917-497-2514

Check us out on Kelly Clarkson https://t.sidekickopen69.com/Ctc/OR+23284/d2nMfk04/JkM2-6qcW6N1vHY6lZ3pXW329_PV42tBPBW3vCgRN841D9RW5McvWj3t73RvN1Q995jb71dFW8JY5QM8mX9w0V4B5Xf7nFvLSW8Rrhh-3CnfNyW875YcF2hcblFW7kKrGm4kBYN6W9gjrtd6BxnW3W8QR2vY7znsncW5ylCzR4C9TyqW6s8LpV8rlpWbN9k2SNlcTtrYW2gVjYR677WgqW1GMFfV3mlcKdW1z7H9R7rhjdBW5J09NK2wN_RYW2VHDn5545d2BW933Dmx6dvsWPN6Jv_nYgHn98W1vcC--5cw9Q4f7XgjC004! Make a gift here https://t.sidekickopen69.com/Ctc/OR+23284/d2nMfk04/Jks2-6qcW69sMD-6lZ3mhW4DZlyJ7PvSC7W3h2Ffq2D2T5RW2Cxs686KSW_KVzsm5x8bNhGPVB_Qdr7V230MW1MHww06zgn9_W34G4HH65Ms2vW18HbN17B0Z7GN5phpP_vb6vpW13BG2c8Br14-W3w7zqt3Mmhl6W3qW-hw5jxgFRW7j494f3cfzrsW7n1jg58w43LMW5w9VXz7l-ZjwW4rS75H1NxrsDW71T7k22-_cmFW7yqr5w6WL6RhW53TVQR2dntDDW7QqZ6V63yW-Hf81tJ-R04 !

Note, I honor and respect boundaries around personal time, self-care, caregiving, and rest. If you receive correspondence from me during a time when you’re engaging in any of the above, please protect your energy and respond when you have the capacity.

On Wed, Aug 16, 2023 at 1:27 PM matt @.***> wrote:

@danthefridgeman https://github.com/danthefridgeman To test on the live airtable you can use this link: https://admin-portal-admin-portal-pr-106.up.railway.app/

Its the same login as the normal website

— Reply to this email directly, view it on GitHub https://github.com/grassrootsgrocery/admin-portal/pull/106#issuecomment-1681009722, or unsubscribe https://github.com/notifications/unsubscribe-auth/AXAWI555FXFHB45QSVRRDW3XVT7I7ANCNFSM6AAAAAA2NAX46E . You are receiving this because you were mentioned.Message ID: @.***>

mattsahn commented 1 year ago

Wow, so cool! The only thing that's now off is this delivery and location info page where I am getting the spinning wheel of death [image: Screen Shot 2023-08-16 at 2.52.47 PM.png]

@danthefridgeman, your screenshot didn't come through, but I presume you mean this part, right? This is the issue @hgreenhut and I were talking about that resurfaced but we know how to fix. Re-opened that issue, #118

image

danthefridgeman commented 1 year ago

Yep that’s what I’m referring to!

And wow yes amazing work to everyone involved!

On Wed, Aug 16, 2023 at 10:16 PM Matt Sahn @.***> wrote:

Wow, so cool! The only thing that's now off is this delivery and location info page where I am getting the spinning wheel of death [image: Screen Shot 2023-08-16 at 2.52.47 PM.png]

@danthefridgeman https://github.com/danthefridgeman, your screenshot didn't come through, but I presume you mean this part, right? This is the issue @hgreenhut https://github.com/hgreenhut and I were talking about that resurfaced but we know how to fix. Re-opened that issue, #118 https://github.com/grassrootsgrocery/admin-portal/issues/118

[image: image] https://user-images.githubusercontent.com/15386131/261177715-e9b0bf5e-5edc-48ce-8757-2e2cecac51f9.png

— Reply to this email directly, view it on GitHub https://github.com/grassrootsgrocery/admin-portal/pull/106#issuecomment-1681509103, or unsubscribe https://github.com/notifications/unsubscribe-auth/AXAWI55J7ML4YLUP4IPTYUTXVV5JLANCNFSM6AAAAAA2NAX46E . You are receiving this because you were mentioned.Message ID: @.***>

--

Dan Zauderer Founder, Grassroots Grocery 917-497-2514

Check us out on Kelly Clarkson http://www.grassrootsgrocery.org/grassroots-on-kelly! Make a gift here http://www.grassrootsgrocery.org/donate!

Note, I honor and respect boundaries around personal time, self-care, caregiving, and rest. If you receive correspondence from me during a time when you’re engaging in any of the above, please protect your energy and respond when you have the capacity.

zuechai commented 1 year ago

I don't have access to Airtable to confirm tests, but this looks great! Really impressive and thorough, @6mp! I'll leave it for @mattsahn or @jasoncavanaugh to approve since I only read code and open the deployment at the moment.

mattsahn commented 1 year ago

I don't have access to Airtable to confirm tests, but this looks great! Really impressive and thorough, @6mp! I'll leave it for @mattsahn or @jasoncavanaugh to approve since I only read code and open the deployment at the moment.

Just sent you a new set of credentials so you can connect to the airtable dev DB

6mp commented 1 year ago

@jasoncavanaugh Thank you so much for your comments, I implemented them into the code and it really cleaned things up!

mattsahn commented 1 year ago

@jasoncavanaugh Thank you so much for your comments, I implemented them into the code and it really cleaned things up!

Wow, that was an epic PR and dialogue/feedback. Love to see the healthy PR culture happening here, @6mp and @jasoncavanaugh!