Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow staff-api to ignore approval workflows for any given type of booking #205

Open
w-le opened this issue Aug 2, 2022 · 10 comments
Open
Assignees
Labels
focus: backend Focus on Back End Development priority: low type: enhancement new feature or request

Comments

@w-le
Copy link
Contributor

w-le commented Aug 2, 2022

In many scenarios, a booking workflow does not require an approval process.
In these cases, frontend apps will create (POST) the booking with .approved=true and that will persist.

HOWEVER, when a booking is EDITED (e.g. moved to a different resource or the start time changes), staff-api will always change .approved to false when processing the PUT:
https://github.com/PlaceOS/staff-api/blob/master/src/controllers/bookings.cr#L230

As a result, the app will show the booking as "Declined", which is not desirable in scenarios where there is no approval process (that is most scenarios).

There should be a staff-api configuration flag (perhaps set in PlaceOS Domain.internals or in staff-api.tenant that defines whether the staff-api expects certain bookings (e.g. of type 'desk' or type 'room') to have an approval workflow or not.

The current default is that all Bookings have an approval workflow, but I believe a better default is that there is no approval workflow (because this is a more common scenario), and one can optionally be enabled with a setting like the above. Though if we implement in this way we would have to add to Release Notes that the new setting to enable approval needs to be added to any existing deployments (with approval workflows) that get upgraded.

When there is no approval workflow configured, then staff-api should not change the booking.approval value on edits.

@w-le w-le added type: enhancement new feature or request priority: medium focus: backend Focus on Back End Development labels Aug 2, 2022
@jeremyw24
Copy link
Contributor

Clear approval flags vs setting to false in staff-api bookings controller.

@chillfox
Copy link
Contributor

Looking at the code I think the app should only show a booking as "Declined" if "rejected" is true

@w-le
Copy link
Contributor Author

w-le commented Aug 12, 2022

ok I'll let @MrYuion and @stakach decide whether to implement this as a front or backend change

@stakach
Copy link
Member

stakach commented Aug 22, 2022

Yeah I agree, rejected == true indicates rejected
approved == false just means it hasn't been approved yet and might not require approval

@w-le
Copy link
Contributor Author

w-le commented Aug 23, 2022

I see it's closed but in the comments it's unclear what the outcome is -

  • Is there no action to be taken now? (closed not an issue)
  • or does it mean frontend changes will be made to address the issue in the OP? (moved to a gh issue in the placeos/user-interfaces repo)

@chillfox
Copy link
Contributor

The front end should be updated to use both the rejected and approved fields to determine the state of a booking instead on only relying on the approved field.

I don't know if an issue has been created in placeos/user-interfaces.

@w-le
Copy link
Contributor Author

w-le commented Aug 23, 2022

ok good thing I asked then, cause there was an intended action for another team, but no replacement issue to track it, so the intended action would've been forever lost. fyi @jeremyw24 I've now created a new issue to replace this one.

@MrYuion
Copy link

MrYuion commented Aug 26, 2022

Hmm... the frontend already uses both for determining the status of a booking.
https://github.com/PlaceOS/user-interfaces/blob/b333879d29258e21e4abd07e3f3492d779329f29/libs/bookings/src/lib/booking.class.ts#L130

@w-le
Copy link
Contributor Author

w-le commented Aug 26, 2022

ok there's disagreement so i'm re-opening until there's a clear agreed action/path to resolution/

@w-le w-le reopened this Aug 26, 2022
@w-le
Copy link
Contributor Author

w-le commented Aug 26, 2022

note that it's not urgent at this stage

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus: backend Focus on Back End Development priority: low type: enhancement new feature or request
Projects
None yet
Development

No branches or pull requests

5 participants