Skip to content

fix: Handle None ECTS#158

Open
Batirro wants to merge 8 commits intomasterfrom
fix/133-fix-grade-fetching-errors
Open

fix: Handle None ECTS#158
Batirro wants to merge 8 commits intomasterfrom
fix/133-fix-grade-fetching-errors

Conversation

@Batirro
Copy link
Member

@Batirro Batirro commented Feb 25, 2026

Important

Fixes None ECTS handling in get_user_courses_ects_safe() and adds tests for various scenarios.

  • Behavior:
    • Introduces get_user_courses_ects_safe() in views.py to handle None ECTS values by setting them to 0.0.
    • Updates get_grades() in views.py to use the safe wrapper function.
  • Tests:
    • Adds test_grade_fetching.py to test filtering of None values, handling of all None terms, empty responses, conversion of string ECTS to float, and mixed valid/None courses.
  • Logging:
    • Logs the number of courses with None ECTS values and sets them to 0.0 in get_user_courses_ects_safe().

This description was created by Ellipsis for 4bfe13f. You can customize this summary. It will automatically update as commits are pushed.

Add get_user_courses_ects_safe that converts string ECTS to float,
replaces missing/invalid values with 0.0 and logs occurrences.
Use the safe wrapper in get_grades and add async tests covering
filtering/conversion, empty responses, and mixed valid/None courses.
Copilot AI review requested due to automatic review settings February 25, 2026 22:01
@Batirro Batirro linked an issue Feb 25, 2026 that may be closed by this pull request
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a defensive wrapper around the USOS “user ECTS points” call to avoid crashes when the API returns None/non-numeric ECTS values, and introduces unit tests meant to cover the new behavior in the grades app.

Changes:

  • Added get_user_courses_ects_safe() and switched get_grades to use it.
  • Added a new test module for the safe ECTS wrapper.
  • Initialized the grades.tests package.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 8 comments.

File Description
grades/views.py Introduces the safe ECTS fetch/normalize helper and routes get_grades through it.
grades/tests/test_grade_fetching.py Adds tests intended to validate handling of None/string ECTS values.
grades/tests/__init__.py Marks grades/tests as a package for test discovery/imports.

Batirro and others added 2 commits February 25, 2026 23:10
Expect None ECTS values to be converted to 0.0 instead of filtered
out. Update AsyncMock usage to mock_client.connection.get and adjust
assertions accordingly.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings February 25, 2026 22:15
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.

Use a more descriptive missing_or_invalid_ects_count and update the info
log accordingly. Also switch the test to SimpleTestCase to avoid
unnecessary
DB setup for an async unit test.
Cover get_user_courses_ects_safe to ensure non-numeric or non-string
ECTS values are converted to 0.0 and do not break the result structure.
@Batirro
Copy link
Member Author

Batirro commented Feb 25, 2026

Niestety jak na razie nie mam pojęcia jak naprawić ten problem z testami, bo lokalnie u mnie przechodzą bez problemu. A według chatu to problem leży w tym, że testy jednostkowe nie są widoczne w CI.

Copilot AI review requested due to automatic review settings February 26, 2026 23:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

@Batirro
Copy link
Member Author

Batirro commented Feb 26, 2026

O testy przeszły. Ogólnie jednak problem leżał w tym że w grades był pusty plik tests.py a ja dodatkowo stworzyłem testy w podfolderze tests, a w unittest w django wywala właśnie taki błąd. Tak czy inaczej teraz działa, dlatego @MoonPrincess06 proszę o review w wolnej chwili. Z góry dzięki :)

@Batirro Batirro changed the title fix: Handle None ECTS and add tests fix: Handle None ECTS Feb 26, 2026
Copy link
Collaborator

@MoonPrincess06 MoonPrincess06 left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Poprawić błędy przy pobieraniu ocen

3 participants