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

Added missing students section for general assemblies #509

Open
wants to merge 11 commits into
base: development
Choose a base branch
from

Conversation

horcsinbalint
Copy link
Member

Description

Added missing students section

Related Issue

#499

Changes

  • Added table for students that should attend general assemblies
  • List students that are missing

@horcsinbalint horcsinbalint requested a review from a team as a code owner April 11, 2024 17:45
deepsource-autofix bot and others added 10 commits April 11, 2024 17:46
This commit fixes the style issues introduced in 2a08067 according to the output
from PHP CS Fixer.

Details: #509
This commit fixes the style issues introduced in 1c93437 according to the output
from PHP CS Fixer.

Details: #509
Passive students are also required to attend general assemblies. To make it a little less painful for the President, they are automatically excused. Their excuse can be removed if needed
@viktorcsimma
Copy link
Contributor

Maybe I miss the point, but wouldn't it be easier to connect this with #482? That way, we would know exactly who was a collegist at that time, and from the status, we would also know whether they were active in the semester.

@horcsinbalint
Copy link
Member Author

horcsinbalint commented Apr 12, 2024

Maybe I miss the point, but wouldn't it be easier to connect this with #482? That way, we would know exactly who was a collegist at that time, and from the status, we would also know whether they were active in the semester.

You are right, my implementation stores whether they were enrolled at the time which could be done easier if #482 was fixed.

@kdmnk
Copy link
Contributor

kdmnk commented Apr 16, 2024

Just took a quick look. Yes, I don't see why storing the users in a new table is necessary. We already have everything for this in the statuses. Just make a new relation between general assemblies and semesters. Anyone who is active and not excused is supposed to be there, right?

@horcsinbalint
Copy link
Member Author

Just took a quick look. Yes, I don't see why storing the users in a new table is necessary. We already have everything for this in the statuses. Just make a new relation between general assemblies and semesters. Anyone who is active and not excused is supposed to be there, right?

Yes, if alumnis are never considered active. Idk, it might be the case. I was not really sure if there could be a case where alumnis / guests have an active status as previously there was a filter in the code for non-alumni collegists. But yes, it seems to be redundant.

Is there a way for us to get the semester for a given date or we should store the semester for the GeneralAssembly object?

@kdmnk
Copy link
Contributor

kdmnk commented Apr 16, 2024

Add a semester_id. It will fit with the new semester behaviour I'm working with.
Statuses and roles are not connected but without human intervention we should not have an active alumni. Worst case, someone will find them in this list and update them so it's not a big deal.

@kdmnk kdmnk changed the title Added missing students section Added missing students section for general assemblies Apr 16, 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.

None yet

3 participants