Skip to content
This repository has been archived by the owner on Mar 27, 2023. It is now read-only.

Initial version of a statistics page #453

Merged
merged 28 commits into from
May 12, 2020
Merged

Initial version of a statistics page #453

merged 28 commits into from
May 12, 2020

Conversation

feeds
Copy link
Collaborator

@feeds feeds commented May 2, 2020 •
edited by Baschdl
Loading

First version of #271

@feeds feeds requested review from Baschdl and josauder May 2, 2020 15:46
@Baschdl Baschdl mentioned this pull request May 2, 2020
Open
2 tasks
@bjrne bjrne self-assigned this May 2, 2020
@Baschdl
Copy link
Collaborator

Baschdl commented May 2, 2020 •
edited
Loading

We need a better user permission system for this. @maltezacharias is on it

@feeds
Copy link
Collaborator Author

feeds commented May 3, 2020

Well, we should open another issue for this, right?

I can easily add another group of 'statsviewers', what is @maltezacharias onto? 😅

@bjrne bjrne added PR:wip Pull request is work in progress and removed PR:ready-for-merge labels May 3, 2020
@Baschdl
Copy link
Collaborator

Baschdl commented May 3, 2020 •
edited
Loading

Yes, he opened #466. Shouldn't prevent merging this PR

@Baschdl Baschdl removed the PR:wip Pull request is work in progress label May 3, 2020
Baschdl
Baschdl suggested changes May 3, 2020
View reviewed changes
Copy link
Collaborator

@Baschdl Baschdl left a comment

Choose a reason for hiding this comment

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

Did you leave out "Deactivated Institutions" on purpose?

backend/apps/accounts/db_stats.py Outdated Show resolved Hide resolved
backend/apps/accounts/db_stats.py Outdated Show resolved Hide resolved
backend/apps/accounts/db_stats.py Outdated Show resolved Hide resolved
backend/apps/accounts/db_stats.py Outdated Show resolved Hide resolved
@feeds
Copy link
Collaborator Author

feeds commented May 4, 2020

I did not know they could deactivate themselves. I can add it if required.

@Baschdl
Copy link
Collaborator

Baschdl commented May 5, 2020

I did not know they could deactivate themselves. I can add it if required.

Now I'm not sure either. Isn’t that part of the basic user functionality? If it's not, why can’t they deactivate themselves if they found someone for now but need more help later?

Can’t test it locally right now.

@bjrne bjrne removed their assignment May 6, 2020
feeds and others added 4 commits May 11, 2020 17:27
Baschdl
Baschdl approved these changes May 12, 2020
View reviewed changes
Copy link
Collaborator

@Baschdl Baschdl left a comment

Choose a reason for hiding this comment

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

Otherwise it's fine

backend/apps/accounts/db_stats.py Outdated Show resolved Hide resolved
@feeds
Copy link
Collaborator Author

feeds commented May 12, 2020

thank youuu :)

@Baschdl Baschdl mentioned this pull request May 12, 2020
Merged
@Baschdl Baschdl merged commit 82cb533 into staging May 12, 2020
bjrne
bjrne reviewed May 13, 2020
View reviewed changes
Copy link
Member

@bjrne bjrne left a comment

Choose a reason for hiding this comment

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

Nooooooooo :D

backend/apps/accounts/db_stats.py
@@ -19,7 +19,7 @@ class DataBaseStats:
stat_count = RegisterList()
stat_list = RegisterList()

# TODO: last X days?
# TODO: last X days? # noqa: T003
Copy link
Member

Choose a reason for hiding this comment

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

This is not the idea @Baschdl 😆
Either its an idea that isn't worth pursuing, or it is a TODO and should get its own issue for keeping track of it :D

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's a PO issue where we just have to wait for input from the rest. Not sure if opening an issue for all TODOs is a good idea. I fear that the issues will get completely messy

backend/apps/accounts/db_stats.py
@@ -46,7 +46,8 @@ def deactivated_accounts(self):
Student.objects.filter(is_activated=False).count(),
)

# TODO: helfende pro bundesland und großstadt
# TODO: helfende pro bundesland und großstadt. Requires
# https://github.com/match4everyone/match4healthcare/issues/492
Copy link
Member

Choose a reason for hiding this comment

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

Same here, the issue is supposed to be about the TODO itself. In this way, nobody will ever check this again. If they were linked on github, one could see more easily whether the dependency issue is closed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can reference this as dependent in 492 but I wouldn't open a separate issue

@bjrne bjrne deleted the statistics-page branch May 13, 2020 11:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Baschdl Baschdl Baschdl approved these changes

@bjrne bjrne bjrne left review comments

@josauder josauder Awaiting requested review from josauder

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@feeds @Baschdl @bjrne