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

Sac points tracking system:tm: #1015

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Sac points tracking system:tm: #1015

wants to merge 17 commits into from

Conversation

stickyPiston
Copy link
Member

@stickyPiston stickyPiston commented Jun 8, 2022

finally (will fix #918)

@Siem2l
Copy link
Contributor

Siem2l commented Jun 13, 2022

Commissions are missing entirely from this PR right now. Each commission should have their own SAC points for being a member of the commission. This was a quick review will do a bit more indepth later on

Copy link
Member

@Riscky Riscky left a comment

Choose a reason for hiding this comment

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

Great to see this PR!
In general it looks really good, well done :)
I left a few comments I'd like to see you address, but apart from that this looks quite ready to merge.
Let me know if you have questions about anything I wrote.

app/views/admin/members/show.html.haml Outdated Show resolved Hide resolved
yarn.lock Outdated Show resolved Hide resolved
config/initializers/sac.rb Outdated Show resolved Hide resolved
@Riscky
Copy link
Member

Riscky commented Jun 19, 2022

Commissions are missing entirely from this PR right now. Each commission should have their own SAC points for being a member of the commission. This was a quick review will do a bit more indepth later on

Depending on the exact implementation details of the committee points, this might be something for a later PR.
If the amount of committee points is equal to the amount of committees, it should be as simple as adding
member.groups.where(category: :committee).count, and that could be added in this PR.
Something more involved should be moved to a later PR in my opinion.
The reasoning being that it would be fairly easy for the board to calculate the committee points by hand, while waiting to merge this PR means that way more work has to be done by the board.
@Jelte-Akker could you give more details here on how this should be calculated?

@Mstiekema
Copy link
Member

I've held some initial talks for SAC and when Joris de Jong and I were talking about Koala implementatio, we also thought the comittees should not be in the MVP. This can easily be calculated if someone lets us know that they want their certificate. In principle students can calculcate it themselves. I think it's important to merge this PR asap, since the new year is around the corner.

@stickyPiston
Copy link
Member Author

All requested changes are implemented and this PR only needs the sac categories to be filled out. Waiting for further actions now

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.

Add SAC Feature to Koala
4 participants