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

fix: track worksheet usage #1828

Merged
merged 6 commits into from
Nov 14, 2023
Merged

fix: track worksheet usage #1828

merged 6 commits into from
Nov 14, 2023

Conversation

SKairinos
Copy link
Contributor

@SKairinos SKairinos commented Nov 9, 2023

This change is Reviewable

Copy link
Member

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @SKairinos)


aimmo/views.py line 223 at r1 (raw file):

            game.save()

        WorksheetUsage.objects.create(

This would create a WorksheetUsage object every time someone watches the game = every time any user clicks Play or enters the URL for the game, teachers & students alike. Just double-checking that is what we want (I thought we mainly wanted to just keep track of when a Game changes Worksheet).

Copy link
Contributor Author

@SKairinos SKairinos left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @faucomte97)


aimmo/views.py line 223 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

This would create a WorksheetUsage object every time someone watches the game = every time any user clicks Play or enters the URL for the game, teachers & students alike. Just double-checking that is what we want (I thought we mainly wanted to just keep track of when a Game changes Worksheet).

I'm aware - it was intentional. I want to capture every time someone uses/plays a game/worksheet. This way, we know which are most used worksheets

Copy link
Member

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @SKairinos)

Copy link
Member

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @SKairinos)

Copy link
Member

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @SKairinos)

Copy link

codecov bot commented Nov 14, 2023

Codecov Report

Merging #1828 (006fbf5) into master (a7574d4) will increase coverage by 4.28%.
Report is 121 commits behind head on master.
The diff coverage is 85.29%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1828      +/-   ##
==========================================
+ Coverage   66.32%   70.60%   +4.28%     
==========================================
  Files         179      190      +11     
  Lines        3661     3950     +289     
  Branches      255      272      +17     
==========================================
+ Hits         2428     2789     +361     
+ Misses       1201     1132      -69     
+ Partials       32       29       -3     
Files Coverage Δ
aimmo-game-worker/simulation/utils.py 100.00% <100.00%> (ø)
aimmo/__init__.py 100.00% <100.00%> (ø)
aimmo/app_settings.py 100.00% <100.00%> (ø)
aimmo/exceptions.py 100.00% <100.00%> (ø)
aimmo/game_manager/__init__.py 100.00% <100.00%> (ø)
aimmo/game_manager/constants.py 100.00% <100.00%> (ø)
aimmo/game_manager/game_secret_manager.py 100.00% <100.00%> (ø)
aimmo/middleware/game_limit_exceeded.py 100.00% <100.00%> (ø)
aimmo/migrations/0003_auto_20160802_1418.py 48.27% <ø> (ø)
aimmo/migrations/0009_add_game_status.py 100.00% <ø> (ø)
... and 45 more

... and 19 files with indirect coverage changes

@SKairinos SKairinos merged commit f1807fb into master Nov 14, 2023
7 of 8 checks passed
@SKairinos SKairinos deleted the worksheet_usage branch November 14, 2023 12:36
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.

None yet

2 participants