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

feat: shift system #94

Closed
wants to merge 89 commits into from
Closed

feat: shift system #94

wants to merge 89 commits into from

Conversation

jofmi
Copy link
Owner

@jofmi jofmi commented Jun 3, 2024

No description provided.

Theophile-Madet and others added 30 commits April 29, 2024 14:51
This reverts commit d1eff25.
Prefixed all collections and fields names with the extension name.
…o avoid reloading the nuxt app when clicking a shift.
Copy link
Contributor

@Theophile-Madet Theophile-Madet left a comment

Choose a reason for hiding this comment

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

I couldn't go much farther without the calendar working, maybe you can present me the changes in the coming days?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice generalisation :)

Comment on lines +118 to +152
// modal: {
// wrapper: "relative z-50",
// inner: "fixed inset-0 overflow-y-auto",
// container:
// "flex min-h-full items-end sm:items-center justify-center text-center",
// padding: "p-4 sm:p-0",
// margin: "sm:my-8",
// base: "relative text-left rtl:text-right overflow-hidden w-full flex flex-col",
// overlay: {
// base: "fixed inset-0 transition-opacity",
// background: "bg-primary/20 dark:bg-gray-800/75",
// transition: {
// enter: "ease-out duration-300",
// enterFrom: "opacity-0",
// enterTo: "opacity-100",
// leave: "ease-in duration-200",
// leaveFrom: "opacity-100",
// leaveTo: "opacity-0",
// },
// },
// background: "bg-white dark:bg-gray-900",
// ring: "",
// rounded: "border border-purple-500 rounded-xl",
// shadow: "",
// width: "sm:max-w-[520px]",
// height: "",
// transition: {
// enter: "ease-out duration-300",
// enterFrom: "opacity-0 translate-y-4 sm:translate-y-0 sm:scale-95",
// enterTo: "opacity-100 translate-y-0 sm:scale-100",
// leave: "ease-in duration-200",
// leaveFrom: "opacity-100 translate-y-0 sm:scale-100",
// leaveTo: "opacity-0 translate-y-4 sm:translate-y-0 sm:scale-95",
// },
// },
Copy link
Contributor

Choose a reason for hiding this comment

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

Robert Martin from "Clean Code" is very, errr, assertive about removing code instead of commenting it out 😛

It makes me crazy to see stretches of code that are commented out. Who knows how old it is? Who knows whether or not it’s meaningful? Yet no one will delete it because everyone assumes someone else needs it or has plans for it.

That code sits there and rots, getting less and less relevant with every passing day. It calls functions that no longer exist. It uses variables whose names have changed. It follows conventions that are long obsolete. It pollutes the modules that contain it and distracts the people who try to read it. Commented-out code is an abomination.

When you see commented-out code, delete it! Don’t worry; the source code control system still remembers it. If anyone really needs it, he or she can go back and check out a previous version. Don’t suffer commented-out code to survive.

https://www.informit.com/articles/article.aspx?p=1334908

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have the same opinion :)

Comment on lines +104 to +107
await updateEvents(
DateTime.fromJSDate(calendar.view.activeStart),
DateTime.fromJSDate(calendar.view.activeEnd),
);
Copy link
Contributor

Choose a reason for hiding this comment

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

I get the following error on those lines when I try to access the calendar. The calendar shows up but is empty.

{"errors":[{"message":"Invalid query. \"_in\" has to be an array of values.","extensions":{"code":"INVALID_QUERY","reason":"\"_in\" has to be an array of values"}}]}

rrule: RRule;
}

export const getShiftOccurrences = async (
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understood correctly, this function hides all the occurrences without an available slot?
I think hiding slots is confusing for normal users, they need to see the structure of the shifts. I would instead show all occurrences and all slots but fade out the unavailable ones a bit or highlight the available ones.

shifts_from: { _lte: to.toISO() },
shifts_status: { _eq: ItemStatus.PUBLISHED },
},
fields: ["*", "shift_slots.*", "shift_slots.shifts_assignments.*"],
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure where that differences come from, but according to my IDE this is invalid and should be

fields: ["*", { shifts_slots: ["*", { shifts_assignments: ["*"] }] }],

I guess yours doesn't show any errors here?

Comment on lines +266 to +300
export const isShiftDurationModelActive = (
durationModel: { shifts_from: string; shifts_to?: string },
atDate?: DateTime,
): boolean => {
return isFromToActive(
DateTime.fromISO(durationModel.shifts_from),
durationModel.shifts_to
? DateTime.fromISO(durationModel.shifts_to)
: undefined,
atDate,
true,
);
};

export const isFromToActive = (
from: DateTime,
to?: DateTime,
atDate?: DateTime,
dateOnly = true,
): boolean => {
if (!atDate) {
atDate = DateTime.now();
}

if (dateOnly) {
from = from.startOf("day");
to = to?.endOf("day");
}

if (from > atDate) {
return false;
}

return !(to && to < atDate);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought I had removed those two since they are now under utils, I guess they got re-added by mistake when merging?
pnpm dev gives warnings about them being defined twice.

@jofmi jofmi closed this Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants