Skip to content

Conversation

@limwa
Copy link
Member

@limwa limwa commented Nov 30, 2024

This PR makes it so uni doesn't try to fetch exams for students with the mobility course or other courses with a null id.

Review checklist

  • Terms and conditions reflect the current change
  • Contains enough appropriate tests
  • If aimed at production, writes a new summary in whatsnew/whatsnew-pt-PT
  • Properly adds an entry in changelog.md with the change
  • If PR includes UI updates/additions, its description has screenshots
  • Behavior is as expected
  • Clean, well-structured code

@codecov
Copy link

codecov bot commented Nov 30, 2024

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 12%. Comparing base (b330fc3) to head (4d0205b).
⚠️ Report is 1373 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff           @@
##           develop   #1402   +/-   ##
=======================================
- Coverage       12%     12%   -0%     
=======================================
  Files          266     266           
  Lines         7218    7240   +22     
=======================================
  Hits           806     806           
- Misses        6412    6434   +22     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@limwa
Copy link
Member Author

limwa commented Nov 30, 2024

I can't test this because I don't have a mobility account.
Even then, it's likely that it might not fix the issue.

@thePeras
Copy link
Member

I can't test this because I don't have a mobility account. Even then, it's likely that it might not fix the issue.

I think the issue is more complicated than this. Did you check the student account I leave at #uni-reports?

@limwa
Copy link
Member Author

limwa commented Nov 30, 2024

I can't test this because I don't have a mobility account. Even then, it's likely that it might not fix the issue.

I think the issue is more complicated than this. Did you check the student account I leave at #uni-reports?

Nop, I'll check it out now

@limwa limwa marked this pull request as draft November 30, 2024 21:46
@thePeras
Copy link
Member

thePeras commented Dec 4, 2024

The solution I propose:
If mobility course is identified, get exams method maps every enrollment and gets is exams by using the same method of ge course exams because the html page is exactly the same.

@pedroafmonteiro
Copy link
Member

Closed due to inactivity. However, we must see this in the future again.

@limwa
Copy link
Member Author

limwa commented Sep 24, 2025

To be honest, it did fix half the problem (course units). The other problem was exams.

@pedroafmonteiro
Copy link
Member

pedroafmonteiro commented Sep 24, 2025

Could you explain what was the problem with the course units please?

@limwa
Copy link
Member Author

limwa commented Sep 24, 2025

International students are enrolled in a "Mobilidade" course in SIGARRA. In that course, they have different course units from different courses from FEUP.

However, "Mobilidade" is not a course that exists in SIGARRA normally, it's a special course and it's ID is null. Because of that, the fetcher throws an exception when parsing that course and, because of that, no course units are fetched from SIGARRA.

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