(feat) add multi-user addition for administrators

[adepssimius]

One of my frequent use cases would be to add flights that my kids and other family members are on with me just so that they have a log of their flights. It is tedious to add a flight for myself, then for my wife, then each kid. Ideally, a user should be able to delegate the addition of flights to another user so that their delegate may add the other user to the flight at the same time as the delegate. I have rearranged some UI elements so that the common "flight" attributes are grouped together, so that the attributes that apply to individual people can be separated.

For non-admin users the only visible change is the UI being rearranged.

For admin users, a set of checkboxes enumerating the users appears between the general flight information and the person specific information. You can set attributes that apply to individual people on your user first, then when you click the checkbox for another user the information that you entered into your user is copied over automatically for convenience. The API was refactored to include the general flight information in the request in addition to an array of users that the flight should also be created for, each with their individual attributes included in the array. The response includes the ID of all flights that were created by thy API and the user is redirected to the first ID, which should be for their own flight that was created.

In the interest of making small incremental changes that add value, I have opted to make this simply a feature that administrators may use. In the future a feature may be implemented to add a delegate to allow your delegate to log flights on your behalf.

I'm happy to discuss additional changes if you want to go another direction. Thanks for your consideration.

[pbogre]

Thanks so much for the PR! I totally agree with the feature you suggest. I feel like we can go one step further and implement the delegate system that you suggested, it can just be a new section in the settings with a checkbox for each user. If a user is selected as your delegate then a row is added to a delegates relationship table with columns user and delegate. Then in the New flight page, the 'Add flight for users' section only displays users for which there is a row in delegates where delegate is the username of the user trying to add a flight.

Since this means creating a whole new table we might look for another way like simply adding a list field delegates to the users table, which might be somewhat dirtier but still works fine.

What do you think?

[adepssimius]

Yep, that's the end goal implementation I had in mind. I went with the half measure as a POC here for a couple reasons:

1. No schema changes required for this implementation (I didn't need to learn the db side of things)
2. No delegate permission management system is required if you don't have a delegate system.

Questions I have about continuing this work:

1. What's your preferred name of the person who grants a delegate permission? Grantor? Principal? This will determine function names so I want to standardize so we have clearly readable code. (getPrincipals(), for instance)
2. I would likely stop and refactor things with a permission service of some kind at this point. We would want to be able to do handy things like figure out if the current user is an admin, get a list of delegates for the current user, and determine if the current user can edit a given flight id (is the flight owner in my list of principals?).
3. Should an admin be able to create or edit for anybody regardless of delegate status or not? (I would think so. If that's the case, can we merge what exists here and build the delegate system in the next push?)

[adepssimius]

Re: adding a list field to the users table, I would prefer to add the pivot table now rather than have to migrate it later. It makes for a much cleaner DB design and cleaner code IMO to not do parsing of a list after retrieval from the DB.

I have also been dealing with a several years old poor architecture choice I made about just using a list in a column in my day job for the last week, so I might be a little bitter :P.

[pbogre]

Yeah I understand why you didn't implement the delegate system in this PR. I'm happy to review & merge this PR when I can and then do the delegate system myself if you don't wanna do the whole DB side which is totally fair.

To answer your questions:

1. I'm pretty happy with delegate, as it perfectly gets the meaning across
2. Figuring out if the current user is an admin should be trivial, because on each page you can access the User as an object in react, and it has an isAdmin attribute. Yes getting a list of delegates for the current user would be needed for the settings section where you pick which users are your delegates. I have to say that i'm not sure about allowing delegates to edit flights, because that creates a lot of complications IMO. It could be done by making the delegated flights somehow store all the users it was delegated for including the delegator (another table?) and then whenever someone edits it it updates for everyone. However this is quite messy (delegation is not transitive or reflexive) so I feel like it's best to stick to flight creation only.
3. Yes I think an admin should have that permission, since they can delete the user anyway they should have control over their flights.

About the list field, yeah I've had to deal with it myself in the past and it's pretty dirty 😅 Table is definitely cleaner

[pbogre]

PS.: you can ignore the failed check because it's just a bug in the CI/CD.

[adepssimius]

1. Delegate is the person who may act on behalf of someone. I'm talking about the opposite relationship. When we go to retrieve the list of users that you may create flights for, we need to call a function that will get the users. I'm thinking that function will be called getPrincipals() or something to that effect. That's the relationship I'm talking about here.

2. I would solve by simply allowing a user who is a delegate to see all of the flights of their principals, similar to how an admin can filter by all users. Edits can be made if you either own the flight or are a delegate of the owner. I don't think abstracting the flight data further with a one to many relationship between a flight and several passenger details is very valuable at the moment. I'm more than happy to not provide edit permissions for delegates in the next iteration though. I'm always on the side of delivering something valuable over delivering only the most valuable thing.

It would be great if this could get merged then I can iterate from here. I created #126 to document the delegate feature. Let me know if you would like changes here.

[pbogre]

I just reviewed your changes and made a bunch of my own. I tried to keep the implementation veri simple, for example by using form data in the New page instead of states that require their own updating etc., and instead it's all handled by a simple parsing function when posting the flights. The backend support was also kept much simpler and it returns the flight ID for only the person that created all the flights.

Also made some small UI adjustments but there will definitely be more of those before the next release, since I'm not so sure about the way the list of username checkboxes looks right now.

[adepssimius]

I don't agree with this direction. If I am a delegate for somebody I am not necessarily on the flight that I am logging with them.

[pbogre]

see pr discussion for reply

[adepssimius]

Thanks for the review and rework. I like the direction you went. I had one note, which you can see above. How would I log a trip for my wife and kid if I did not go, for instance?

[adepssimius]

Ah, it also seems that your refactor did not bring over the automatic field copy functionality that I had set up. Previously when you had filled out individual traveller information then you added another traveller, all of the information that you had filled out would be copied to the additional users by default. I prefer simpler code over having extra features so no problem. Is this a direction that you would like to go? It's a handy way to not check a bunch of the same boxes.

[pbogre]

Regarding the first comment, I made the decision to not support creating flights for users when you don't have that flight yourself because I see this feature as something to be used only when everyone is boarding the same flight and then you just have one of you fill out the information for everyone. Instead if one of those users is taking a flight for themselves, they should be the ones logging it on their own account. Otherwise having one account be the one adding all the flights for all users the multiple-users functionality is kind of lost and the information could simply be an extra field or tag on the flight.

Anyway this is just how I envisioned this feature, if you think it useful to be able to add flights for other users from your own account, I have no problem adding it as it is quite simple. I just wanted to let you know my reasoning behind that.

Regarding the field copy, I do agree that it was a useful feature on second thought. I will try to implement it back, but if I find that the implementation introduces too much code complexity (I think retrieving the values of the fields might be tricky without using states, which introduces a lot of complexity).