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

schema: Change as needed for the new rotations #193

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

nilmerg
Copy link
Member

@nilmerg nilmerg commented May 6, 2024

rotation

Holds the configuration of a specific rotation.

priority: As the comment states, the lower the more important. Meaning: The shifts described by a rotation with a higher priority (i.e. lower integer) overrides others with a lower one:

image

weekends: 2
anytime: 1
dayshift: 0

mode: The configuration type. It describes the pattern the timeperiod entries, associated to each member, have.

rotation_member

Maps a member of a rotation to a specific contact or contactgroup.

position: Defines the order the member placed at inside the rotation.

image

Though, this is probably not of interest to the daemon. As timeperiod_entry also describes it.

timeperiod_entry

Every member of a rotation has at least one entry associated with it inside the timeperiod of a rotation.

The order of a rotation's members is incorporated in the definition of their entries here.

The daemon just has to interpret them, based on the priority of their rotation, to know who's on duty. Look at the first screenshot again, as the Result is what the intended effect should be.

@nilmerg nilmerg self-assigned this May 6, 2024
@cla-bot cla-bot bot added the cla/signed CLA is signed by all contributors of a PR label May 6, 2024
@nilmerg nilmerg force-pushed the schema-for-rotation-configuration branch 5 times, most recently from 4ba93ab to 5f7d8ff Compare May 8, 2024 14:51
@nilmerg nilmerg force-pushed the schema-for-rotation-configuration branch from 5f7d8ff to ff76b83 Compare May 10, 2024 11:36
@nilmerg nilmerg marked this pull request as ready for review May 10, 2024 11:37
@julianbrost
Copy link
Collaborator

as needed for the new rotations

Is there an issue in the web project that explains how those are supposed to work that you can reference here?

How is all of this supposed to be interpreted? What needs more explanation: rotation.priority1, rotation.mode and rotation_member.position (especially: how do we know the current position?)

Footnotes

  1. I can imagine it based on mock-ups I was shown before, it should be written down nonetheless.

@nilmerg
Copy link
Member Author

nilmerg commented May 10, 2024

Updated the description. The changes in Web are already in a state, that they are fully functional with these changes applied.

@julianbrost
Copy link
Collaborator

mode: The configuration type. It describes the pattern the timeperiod entries, associated to each member, have.

Is rotation.mode in any way relevant to derive the final schedule? Or is all of that information (also) contained in the individual entries?

Is it correct as a very high-level summary that with this, a new schedule is now a list of what previously was a schedule (and is now called rotation)? So to resolve the schedule to recipients at a specific time, evaluate each rotation like an old schedule was evaluated. Do this for each rotation in the order given by rotation.priority until the one gives a result.

@nilmerg
Copy link
Member Author

nilmerg commented May 10, 2024

Or is all of that information (also) contained in the individual entries?

Yes. Think of a 24/7 rotation with a handoff every 2 weeks and two members, to be associated with entries that have a duration of 2 weeks, each with a start date, both 2 weeks apart and a recurrence frequency of 4 weeks.

Is it correct as a very high-level summary...

I'd rephrase

in the order given by rotation.priority

with

in ascending order based on rotation.priority

@julianbrost
Copy link
Collaborator

As just discussed with @nilmerg: this change makes the timeperiod table irrelevant for the daemon for the moment (it can select everything needed following the schedule -> rotation -> rotation_member -> timeperiod_entry relations). It's kept though for possible future use (having a holiday time period that can be used to restrict per-user availability for example) and also as it seems to allow Web to perform some more efficient updates.

@julianbrost julianbrost self-assigned this May 14, 2024
@nilmerg nilmerg force-pushed the schema-for-rotation-configuration branch from b212483 to 0cbee4d Compare May 15, 2024 10:30
CREATE TABLE schedule_member (
schedule_id bigint NOT NULL REFERENCES schedule(id),
timeperiod_id bigint NOT NULL REFERENCES timeperiod(id),
CREATE TABLE rotation_member (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Table rotation_member must be created before timeperiod_entry.
Same in upgrade file.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment on lines 108 to 111
-- Set in case the first_handoff was in the past during creation of the rotation.
-- It is essentially the creation time of the rotation.
-- Used by Web to avoid showing shifts that never happened
actual_handoff bigint,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is essentially the creation time of the rotation.

Even if first_handoff is set to be in the future when creating the rotation?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, in that case it's NULL

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you always set it? That would actually make it useful as it would allow to easily determine the currently active version of the rotation (the one with the largest value of actual_handoff, that is at or before the current time).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

first_handoff should suffice for this job, shouldn't it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only if the switchover to the newer version happens at a fixed time like 00:00 UTC. Otherwise, it misses timezone and/or time of day information.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed it, by the way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla/signed CLA is signed by all contributors of a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants