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

Adjust table with student comparisons #46

Merged
merged 2 commits into from
Feb 13, 2024
Merged

Conversation

mikaelGusse
Copy link
Contributor

@mikaelGusse mikaelGusse commented Feb 9, 2024

Removed duplicate pairs from the table of student comparisons. In the process changed how the table is rendered

Description

What?

  • Changed how the table in the exercise view is rendered. Duplicate pairs are removed.
  • Replaced outdated unit tests in matcher/tests.py and data/tests.py to test new changes

Why?

  • An improvement suggested in issue 31.
  • Improves overall usability by not cluttering the view with the same comparisons multiple times

How?

  • Changed how the function top_comparisons in models.py works, to return the list of comparisons in a set instead of a dictionary of lists, thus removing the duplicates. Additionally rewrote the django template for the table to support this new data structure.

Fixes #31 (partly)

Testing

Remember to add or update unit tests for new features and changes.

What type of test did you run?

  • Accessibility test using the WAVE extension.
  • Django unit tests.
  • Selenium tests.
  • Other test. (Add a description below)
  • Manual testing.

Tested with 5 submissions and 100 submissions locally. Submissions were "fake" and essentially identical. Testing with real data would be preferred before merging.

Did you test the changes in

  • Chrome
  • Firefox
  • This pull request cannot be tested in the browser.

Think of what is affected by these changes and could become broken

Translation

Programming style

  • Did you follow our style guides?
  • Did you use Python type hinting in all functions that you added or edited? (type hints for function parameters and return values)

Have you updated the README or other relevant documentation?

  • documents inside the doc directory.
  • README.md.
  • Aplus Manual.
  • Other documentation (mention below which documentation).

Is it Done?

  • Reviewer has finished the code review
  • After the review, the developer has made changes accordingly
  • Customer/Teacher has accepted the implementation of the feature

Clean up your git commit history before submitting the pull request!

@mikaelGusse mikaelGusse changed the title Adjusted table with student comparisons Adjust table with student comparisons Feb 9, 2024
@mikaelGusse mikaelGusse force-pushed the master branch 5 times, most recently from df3f1b8 to ad9fbe6 Compare February 12, 2024 12:35
@mikaelGusse mikaelGusse marked this pull request as ready for review February 12, 2024 13:25
@ihalaij1 ihalaij1 self-requested a review February 12, 2024 14:01
@ihalaij1 ihalaij1 self-assigned this Feb 12, 2024
Removed duplicate pairs from the table of student comparisons. In the process changed how the table is rendered
Created new tests to replace outdated ones.
Copy link
Contributor

@ihalaij1 ihalaij1 left a comment

Choose a reason for hiding this comment

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

Excellent!

I reviewed this with Mikael face-to-face.

@ihalaij1 ihalaij1 merged commit 57f1f0b into apluslms:master Feb 13, 2024
@ihalaij1 ihalaij1 mentioned this pull request Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Usability in reports
2 participants