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

Create quiz progress on quiz answers submit #7136

Merged
merged 13 commits into from
Sep 4, 2023

Conversation

merkushin
Copy link
Member

Resolves #7050

We create the quiz progress when start the lesson. When the user submits their answers we implicitly start the lesson.
Here we make sure we have a quiz progress or try to create one.
We don't check the feature flag as it is safe to create quiz progress for comments-based version: it is the same lesson progress there.

Proposed Changes

  • Create quiz progress on lesson progress creation if there is a quiz related to the lesson.

Testing Instructions

  1. Enable tables_based_progress.
  2. Start a lesson (or go directly to a quiz and submit your answers).
  3. Check sensei_lms_progress has quiz progress for the quiz. In case it was created on the lesson start in-progress status expected, on the quiz answers submit depends on quiz settings: could be ungraded, passed, failed, graded.

Pre-Merge Checklist

  • PR title and description contain sufficient detail and accurately describe the changes
  • Acceptance criteria is met
  • Decisions are publicly documented
  • Adheres to coding standards (PHP, JavaScript, CSS, HTML)
  • All strings are translatable (without concatenation, handles plurals)
  • Follows our naming conventions (P6rkRX-4oA-p2)
  • Hooks (p6rkRX-1uS-p2) and functions are documented
  • New UIs are responsive and use a mobile-first approach
  • New UIs match the designs
  • Different user privileges (admin, teacher, subscriber) are tested as appropriate
  • Code is tested on the minimum supported PHP and WordPress versions
  • User interface changes have been tested on the latest versions of Chrome, Firefox and Safari
  • "Needs Documentation" label is added if this change requires updates to documentation
  • Known issues are created as new GitHub issues

@merkushin merkushin added this to the 4.16.2 milestone Aug 28, 2023
@merkushin merkushin requested a review from m1r0 August 28, 2023 23:00
@merkushin merkushin self-assigned this Aug 28, 2023
@codecov
Copy link

codecov bot commented Aug 28, 2023

Codecov Report

Merging #7136 (12f016a) into trunk (05932d8) will increase coverage by 0.13%.
Report is 23 commits behind head on trunk.
The diff coverage is 87.61%.

Impacted file tree graph

@@             Coverage Diff              @@
##              trunk    #7136      +/-   ##
============================================
+ Coverage     49.36%   49.50%   +0.13%     
- Complexity    10546    10582      +36     
============================================
  Files           575      578       +3     
  Lines         44537    44629      +92     
  Branches        402      402              
============================================
+ Hits          21987    22092     +105     
+ Misses        22223    22210      -13     
  Partials        327      327              
Files Changed Coverage Δ
...d-jobs/class-sensei-scheduler-action-scheduler.php 83.33% <0.00%> (ø)
includes/class-sensei-utils.php 52.44% <ø> (ø)
includes/class-sensei.php 23.14% <0.00%> (-0.28%) ⬇️
includes/class-sensei-quiz.php 59.40% <69.56%> (+0.27%) ⬆️
...ternal/action-scheduler/class-action-scheduler.php 100.00% <100.00%> (ø)
...er/migrations/class-student-progress-migration.php 81.63% <100.00%> (+0.73%) ⬆️
...nt-progress/jobs/class-migration-job-scheduler.php 100.00% <100.00%> (ø)
...rnal/student-progress/jobs/class-migration-job.php 100.00% <100.00%> (ø)
...itories/class-quiz-progress-repository-factory.php 100.00% <100.00%> (ø)
...al/student-progress/tools/class-migration-tool.php 100.00% <100.00%> (+68.96%) ⬆️

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ebc6e89...12f016a. Read the comment docs.

@@ -13,6 +13,9 @@
<directory name="widgets" />
</ignoreFiles>
</projectFiles>
<stubs>
<file name="sensei-lms.php" />
Copy link
Member Author

Choose a reason for hiding this comment

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

We had lots of errors like Undefined function Sensei(). I encountered it in this PR as well.
It looks like Psalm for some reason can't discover some entities via reflection. In these cases, I found a recommendation, put files containing those entities into <stubs>.
Another approach might be to create config/psalm/psalm-stubs.php and define functions like Sensei() there (without implementation) and include these stubs in config like I did with above.

@m1r0
Copy link
Member

m1r0 commented Aug 31, 2023

Hm, I wasn't able to reproduce this.

I've tried to create a fresh course with a lesson and a quiz. After taking the course and the quiz, it didn't create a quiz progress entry. Did I do something wrong?

QUZkUE.mp4

@merkushin
Copy link
Member Author

@m1r0 you're right, my bad. Found a few issues. Looks like I tested on stale data.
Fixed it here: ee6a628

@m1r0
Copy link
Member

m1r0 commented Aug 31, 2023

Thanks, I see the quiz progress but it is added even if the lesson doesn't have a quiz. Is that expected?

iHLs69.mp4

@merkushin
Copy link
Member Author

Thanks @m1r0.
I updated the PR to take into account only published quizzes.
However, I'm still puzzled with a question when the publishing happens.

@m1r0
Copy link
Member

m1r0 commented Sep 1, 2023

Nice! Now it creates the quiz progress only when there is a quiz for that lesson. Related to that, does it make more sense to create the quiz progress when you open the quiz itself, similar to how the progress works for lessons?

On a side note, psalm and the linter are failing.

@merkushin
Copy link
Member Author

merkushin commented Sep 1, 2023

Related to that, does it make more sense to create the quiz progress when you open the quiz itself, similar to how the progress works for lessons?

Makes sense for me. Updated here: fc2bad4

I removed creation of the quiz progress from the lesson starting function as it leads to a conflict.

Copy link
Member

@m1r0 m1r0 left a comment

Choose a reason for hiding this comment

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

Nice! 💯

@merkushin merkushin merged commit 164ea6e into trunk Sep 4, 2023
24 checks passed
@merkushin merkushin deleted the fix/create-quiz-progress-on-submit branch September 4, 2023 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Store the quiz progress entry when using the tables
2 participants