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

Learning time does not include all attempts #478

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

hemant10yadav
Copy link
Contributor

@hemant10yadav hemant10yadav commented Jan 23, 2025

Technical Summary

Implemented functionality to allow users to complete the learn module multiple times. This enables capturing the learning time for each module. CCCT-638

Safety Assurance

Automated test coverage

Comprehensive test coverage has been added to ensure that a completed module is created for every submission and that only unique modules are sent to the device. This guarantees accurate progress calculation and eliminates redundancy.

Labels & Review

  • The set of people pinged as reviewers is appropriate for the level of risk of the change

@pxwxnvermx
Copy link
Contributor

@hemant10yadav Are you planning to get this PR QA'ed?


if not created:
raise ProcessingError("Learn Module is already completed")
if completed_modules:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will need logic to make sure we dont create duplicates if the same form is forwarded twice (as happened with visits). Ideally that would be enforced at the DB level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added 609d76a

@@ -90,6 +92,59 @@ def test_form_receiver_learn_module_create(
).exists()


@pytest.mark.django_db
def test_form_receiver_multiple_module_submissions(
Copy link
Member

Choose a reason for hiding this comment

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

Making this test use parameters might make it easy to understand various cases.

@hemant10yadav
Copy link
Contributor Author

@hemant10yadav Are you planning to get this PR QA'ed?

Yeah I have raised the QA form for it.

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.

4 participants