Allow setting default and individual SU budgets for project members #437
Conversation
This commit introduces the functionality to set default and individual SU budgets for project members. It includes the following changes: - Added models ChargeBudget to track individual SU budgets for users in projects. - Implemented set_budget_for_user_in_project function to set SU budgets for individual users in projects. - Added form inputs to allow setting default SU budgets for all non-manager users in a project. - Modified the view_project template to display current SU usage and budget inputs for project members. - Implemented backend logic to handle setting default and individual SU budgets in the view_project view. These changes enable project managers to manage SU budgets more effectively within projects.
There was a problem hiding this comment.
I have a few concerns about this implementation:
- I think enforced_by should be removed, as I don't think it's ever exposed to users or would be particularly useful.
- Projects should probably have a "default SU budget" field, and inside
_check_usage_against_user_budget, if there is no ChargeBudget, it should instead use the project's default SU budget.
| def _check_usage_against_user_budget(self, user, allocation, new_charge): | ||
| user_budget = ChargeBudget.objects.get(user=user, project=allocation.project) | ||
| left = user_budget.su_left() | ||
| if left - new_charge < 0: |
There was a problem hiding this comment.
I think if left < new_charge would be more clear.
| <p class="su-budget-display" style="display: inline-block">{{ u.su_budget }}</p> | ||
| </div> | ||
| <div class="col-xs-5 col-sm-5 col-lg-5"> | ||
| <input |
There was a problem hiding this comment.
How usable is this? It seems like it would be tricky to precisely set the slider. Also steps of 1000 is a lot, since our allocations only have 20000 SUs, and a class might have more than 20 people.
There was a problem hiding this comment.
Maybe 500 would be sufficient step.
Nice. These are very useful comments. Thanks. I will work on it |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
allocations/models.py
Outdated
| from django.conf import settings | ||
| from django.db import models | ||
| from django.db.models import ExpressionWrapper, F, Sum, functions | ||
| from django.core.validators import MaxValueValidator, MinValueValidator |
There was a problem hiding this comment.
[flake8] <401> reported by reviewdog 🐶
'django.core.validators.MaxValueValidator' imported but unused
projects/models.py
Outdated
| @@ -12,6 +12,7 @@ | |||
| from django.utils import timezone | |||
| from django.utils.translation import gettext_lazy as _ | |||
| from django.contrib.sites.models import Site | |||
| from django.core.validators import MaxValueValidator, MinValueValidator | |||
There was a problem hiding this comment.
[flake8] <401> reported by reviewdog 🐶
'django.core.validators.MaxValueValidator' imported but unused
AnishReddyRavula
force-pushed
the
user_su_budgeting
branch
from
ca75c7d to
ee1635b
Compare
AnishReddyRavula
force-pushed
the
user_su_budgeting
branch
from
ee1635b to
129c009
Compare
AnishReddyRavula
force-pushed
the
user_su_budgeting
branch
from
129c009 to
0099c9a
Compare
| role, scopes = keycloak_client.get_user_project_role_scopes( | ||
| lease_eval.user.username, lease_eval.project.charge_code | ||
| ) | ||
| if role == 'member': |
| elif "default_su_budget" in request.POST: | ||
| portal_project.default_su_budget = request.POST["default_su_budget"] | ||
| portal_project.save() | ||
| for u in users: |
There was a problem hiding this comment.
I think for consistency, this should be removed to the balance service check. That way, all of the chargebudget objects are created in the same place, when the user first tries to use SUs.
projects/views.py
Outdated
| else: | ||
| su_budget_value = su_budget.su_budget | ||
| user["su_budget"] = int(su_budget_value) | ||
| user["su_used"] = ChargeBudget( |
There was a problem hiding this comment.
It's a little confusing that this creates a ChargeBudget object, since it's not actually getting it from the database? Insteade maybe current_usage should be a static/util method somewhere?
Refactors the charge calculation logic in the allocations/models.py file for clarity. It introduces a utility function in balance_service/utils/su_calculators.py to calculate total SU usage for users in projects. Additionally, project budget enforcement is implemented. When adding users to projects in projects/membership.py, their budgets are updated to match the project's default budget. The projects/views.py file is updated to delete user budgets if they are assigned the manager role to ensure budget enforcement.
This commit introduces the functionality to set default and individual SU budgets for project members. It includes the following changes:
default_su_budgetto Project model to enforce the budget for all other membersThese changes enable project managers to manage SU budgets more effectively within projects.
Docs PR