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

Major overhaul #197

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

Major overhaul #197

wants to merge 22 commits into from

Conversation

mhvis
Copy link
Member

@mhvis mhvis commented Oct 25, 2021

All changes, in no particular order:

  • Diners contact info: e-mail and telephone number. A user can enter their contact info if they agree with the privacy implications, if they do so the contact info will be shown to everyone who subscribed to the same dining lists. It is shown on the diners page.
  • Redesign of the following pages:
    • Dining info
    • Dining overview
    • Diners list (stats as buttons instead of checkboxes which save directly)
  • Dining list deletion: it is only possible to delete when no diners are signed up. This prevents that the site has to reimburse the kitchen cost of all diners and notify them, instead the cook has to do this explicitly first by removing all diners. The edit page includes a small description on how to cancel or delete a dining list.
  • Grocery cost splits: a more explicit way to split grocery costs over diners. When a cook splits the costs, all diners will be notified with the payment link and contact info. Diners can optionally choose to pay using their account balance, if the cook has enabled this option.
  • Removed the negative balance exception for Quadrivium (!). This makes checking for sufficient balance more transparent, we no longer need to deal with and allow (potentially large) negative balances for Quadrivium members. The new approach is that Quadrivium members can access a page where they can upgrade their balance by specifying an amount and checking a box to confirm. The Quadrivium treasurer has access to a page which shows the total upgraded amount for each user and can debit those users.
  • Removed the "news and updates" page because the content is outdated and more text means less incentive for users to actually read it. Instead if we need to inform the users of new features or clarify unclear features we should just add a notice at the appropriate places (near the actual feature) and later remove the notice when no longer necessary.
  • Update Bootstrap, Font Awesome, Python libraries.
  • Bugfixes (see linked issues)

To do for merging:

  • Need to inform the Quadrivium treasurer on the new invoicing approach. (Maarten)
  • Need to check if board agrees with the ability to pay using account balance. (This ability can be easily turned off)
  • The PR became a bit too huge, some of these changes may not be desired. I can undo unwanted changes in this PR or split them to a separate PR.

@mhvis mhvis linked an issue Oct 28, 2021 that may be closed by this pull request
@mhvis mhvis linked an issue Nov 2, 2021 that may be closed by this pull request
@mhvis mhvis marked this pull request as ready for review April 9, 2022 22:28
@LenaWil
Copy link
Collaborator

LenaWil commented Dec 21, 2022

Is this one still alive?

@LenaWil
Copy link
Collaborator

LenaWil commented Dec 21, 2022

Removed the negative balance exception for Quadrivium (!). This makes checking for sufficient balance more transparent, we no longer need to deal with and allow (potentially large) negative balances for Quadrivium members. The new approach is that Quadrivium members can access a page where they can upgrade their balance by specifying an amount and checking a box to confirm. The Quadrivium treasurer has access to a page which shows the total upgraded amount for each user and can debit those users.

I am not the Q treasurer, but the bill is often sent at the end of the year, right?
How exactly would this make a difference? Instead of negative balances, you now just get negative balances, but visible in a different place. It also just seems, as a Q member, but not a treasurer, just more hassle to upgrade the balance every time, where it’s now just done automatically.

Edit: although I do have to admit, I haven’t read the code yet, maybe I have the wrong idea on how it works.

@mhvis
Copy link
Member Author

mhvis commented Dec 22, 2022

Is this one still alive?

It became too big to merge and I'm not really active anymore with the project. But I'll try to split this into smaller PRs.

Removed the negative balance exception for Quadrivium (!). This makes checking for sufficient balance more transparent, we no longer need to deal with and allow (potentially large) negative balances for Quadrivium members. The new approach is that Quadrivium members can access a page where they can upgrade their balance by specifying an amount and checking a box to confirm. The Quadrivium treasurer has access to a page which shows the total upgraded amount for each user and can debit those users.

I am not the Q treasurer, but the bill is often sent at the end of the year, right? How exactly would this make a difference? Instead of negative balances, you now just get negative balances, but visible in a different place. It also just seems, as a Q member, but not a treasurer, just more hassle to upgrade the balance every time, where it’s now just done automatically.

Edit: although I do have to admit, I haven’t read the code yet, maybe I have the wrong idea on how it works.

It's a semantic difference. Currently each time money is withdrawn a check has to be done if that person is a member of Q, to check if the balance can be negative. It is nice to get rid of that check, for instance using a more explicit upgrade form. Then code that deals with money does not have to take negative balances into account. But if you have a better idea than a form, let me know. Having it more explicit also makes it easier for members and the treasurer to understand. And if there would be multiple associations that uses invoicing, it allows the person to choose the association if they are member of both.

@mhvis mhvis added the invalid This doesn't seem right label Dec 22, 2022
@LenaWil
Copy link
Collaborator

LenaWil commented Dec 22, 2022

for instance using a more explicit upgrade form.

Again, haven’t tested the code so far, but it would be great if there is an option to just select Q or other associations that allow that in the settings form and that form be submitted in automatically for the members for members of Q, so I don’t have to up my balance every time I want to subscribe to a dining list, and/or don’t have money in there that doesn’t need to be there.
(If I leave Scala and there is still money in there, that has been on my Q-bill, I would have to get it back somehow.)

@LenaWil
Copy link
Collaborator

LenaWil commented Dec 22, 2022

But I'll try to split this into smaller PRs.
Thanks!

@mhvis
Copy link
Member Author

mhvis commented Dec 22, 2022

Again, haven’t tested the code so far, but it would be great if there is an option to just select Q or other associations that allow that in the settings form and that form be submitted in automatically for the members for members of Q, so I don’t have to up my balance every time I want to subscribe to a dining list, and/or don’t have money in there that doesn’t need to be there. (If I leave Scala and there is still money in there, that has been on my Q-bill, I would have to get it back somehow.)

An automatic balance increase for Q members at sign up time would be a way to make it easier. The goal is to make the Q balances behave the same way as the other associations in the background, and directly move the negative balance to a separate boekhoudaccount for Quadrivium. But that might indeed be possible to do automatically without an explicit form, so that the balance for Q members just remains 0.

However as I'm less active I probably won't write that. And most of this PR including this Q change will probably not make it to the master branch.

@mhvis mhvis added wontfix This will not be worked on and removed invalid This doesn't seem right labels Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
2 participants