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

Budget summary per committee and budget line #206

Merged
merged 6 commits into from
Nov 7, 2024
Merged

Budget summary per committee and budget line #206

merged 6 commits into from
Nov 7, 2024

Conversation

Mr-salto
Copy link
Contributor

re implementation of summary of expenses per committee and budget line

Copy link
Member

@RafDevX RafDevX left a comment

Choose a reason for hiding this comment

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

Haven't actually reviewed it properly because I'm not on my computer; this is just the obvious what I noticed while scrolling through without reading

@@ -216,6 +216,7 @@ def save_user_profile(sender, instance, **kwargs):
instance.profile.save()



Copy link
Member

Choose a reason for hiding this comment

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

Remove extra line

compose.yaml Outdated
@@ -48,3 +54,5 @@ services:
# Melvin here since he's treasurer at the time of writing.
KTH_ID: melvinj

volumes:
postgres_data:
Copy link
Member

Choose a reason for hiding this comment

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

Files should end with a newline

stats/urls.py Outdated
Comment on lines 12 to 13


Copy link
Member

Choose a reason for hiding this comment

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

Remove extra lines

stats/views.py Outdated
@@ -46,29 +46,51 @@ def index(request):
def summary(request):
if request.method == "POST":
body_data = json.loads(request.body)
# we tried getting it to work with ids, but it didn't,
# so we're using names for now
print("body = " , body_data)
Copy link
Member

Choose a reason for hiding this comment

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

Remove debug print

stats/views.py Outdated
).all()

Copy link
Member

Choose a reason for hiding this comment

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

Remove extra tab

stats/views.py Outdated
def sec_cost_centres(request):
if request.method == "POST":
body_data = json.loads(request.body)
print("body = " , body_data)
Copy link
Member

Choose a reason for hiding this comment

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

Remove debug print

stats/views.py Outdated
sec_cost_centres.add(expense_part.secondary_cost_centre)
# Optionally add the expense part details to the response

print("sec cost centres = " ,sec_cost_centres)
Copy link
Member

Choose a reason for hiding this comment

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

Remove debug print

stats/views.py Outdated
def budget_lines(request):
if request.method == "POST":
body_data = json.loads(request.body)
print("body = " , body_data)
Copy link
Member

Choose a reason for hiding this comment

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

Remove debug print

stats/views.py Outdated
expense_queryset = models.Expense.objects.all()
# expense_queryset is a list of expenses

print(expense_queryset)
Copy link
Member

Choose a reason for hiding this comment

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

Remove debug print

style="width: 100%;"
:disabled="expense_parts.length > 1"
>
<option value="">- Year -</option>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<option value="">- Year -</option>
<option value="">- Välj år -</option>

@@ -17,6 +17,7 @@ services:
- 8000:8000
volumes:
- ./media:/app/media
- ./templates:/app/templates
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- ./templates:/app/templates

I guess it should be written in the readme, but this is already handled if you start it with docker compose up --watch

compose.yaml Outdated
Comment on lines 45 to 47
ports:
- 5432:5432

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ports:
- 5432:5432

stats/views.py Outdated
@@ -80,6 +101,94 @@ def summary(request):
return Response(status=status.HTTP_404_NOT_FOUND)


@csrf_exempt
def sec_cost_centres(request):
if request.method == "POST":
Copy link
Member

Choose a reason for hiding this comment

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

This should be a GET request

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to make a POST as every secondary cost center is dependent on the primary cost center. Maybe there is an easier way to do it with a GET.

Copy link
Member

Choose a reason for hiding this comment

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

You can for instance use query parameters to send along data, if that's what you mean

stats/views.py Outdated

@csrf_exempt
def budget_lines(request):
if request.method == "POST":
Copy link
Member

Choose a reason for hiding this comment

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

Also GET

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same reason

<th>År</th>
<th> Nämnder </th>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<th> Nämnder </th>
<th>Resultatställe</th>

:key="index"
:value="costCentre"
>
{% verbatim %}
Copy link
Member

Choose a reason for hiding this comment

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

This works, but there is also the v-text property

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't know, thank you !

compose.yaml Outdated
Comment on lines 38 to 39
volumes:
- postgres_data:/var/lib/postgresql/data
Copy link
Member

Choose a reason for hiding this comment

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

I don't see a reason to add this as data is persisted as long as you don't run docker compose down and it hides the data for those who already have something in their local databases

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't know and wanted to be sure that the fake data I inserted for tests purposes would be persistent.

compose.yaml Outdated
@@ -47,4 +50,4 @@ services:
# Since we're using the real pls, it's nice to use some user with a lot of privileges. Putting
# Melvin here since he's treasurer at the time of writing.
KTH_ID: melvinj

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

stats/views.py Outdated
Comment on lines 45 to 47
@require_GET
def summary(request):
if request.method == "POST":
body_data = json.loads(request.body)
# we tried getting it to work with ids, but it didn't,
# so we're using names for now
if request.method == "GET":
Copy link
Member

Choose a reason for hiding this comment

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

Aren't these two lines synonymous?

activeCommittees: function() {
return this.committees.filter(c => !c.inactive);
},
el: '#content',
Copy link
Member

Choose a reason for hiding this comment

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

(can't believe I'm saying this) go back to 5 space indentation; easier for diff

budgetLine: '',
amount: 0,
}],
totalSum: null,
Copy link
Member

Choose a reason for hiding this comment

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

Confirm whether this is still needed

stats/views.py Outdated
body_data = json.loads(request.body)
# we tried getting it to work with ids, but it didn't,
# so we're using names for now

expense_parts = None
Copy link
Member

Choose a reason for hiding this comment

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

Fix indentation in all of these

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