Changing uc link to open in app page #1035

adriannalmeida · 2023-11-17T15:47:55Z

Closes #913

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 · 2023-11-17T15:52:14Z

Codecov Report

Merging #1035 (2d0c6c2) into develop (2e17df2) will decrease coverage by 0%.
Report is 1 commits behind head on develop.
The diff coverage is 35%.

Additional details and impacted files

@@           Coverage Diff           @@
##           develop   #1035   +/-   ##
=======================================
- Coverage       16%     15%   -0%     
=======================================
  Files          228     228           
  Lines         6888    6899   +11     
=======================================
- Hits          1083    1020   -63     
- Misses        5805    5879   +74

bdmendes

Looking good! Left a few comments that should be easy to address.

uni/lib/view/schedule/widgets/schedule_slot.dart

bdmendes

Alright, some SDK/git issues to address.
The main point of the feature is done, though!

uni/lib/generated/intl/messages_all.dart

uni/lib/generated/intl/messages_en.dart

uni/lib/view/schedule/widgets/schedule_slot.dart

bdmendes

Good work!

uni/lib/generated/intl/messages_en.dart

uni/lib/view/schedule/widgets/schedule_slot.dart

bdmendes

It's a bit weird that we're creating a sentinel course unit if we can't find one. I think we should simply use something like findOrNull (if that is the correct name) and simply display an error.

limwa

Great work, I just have a few suggestions I thought would be useful!

uni/lib/l10n/intl_en.arb

uni/lib/l10n/intl_pt_PT.arb

limwa · 2024-02-23T13:09:01Z

uni/lib/view/schedule/widgets/schedule_slot.dart

+    final courseUnits =
+        Provider.of<ProfileProvider>(context, listen: false).state!.courseUnits;
+    final notFound =
+        CourseUnit(abbreviation: 'NF', name: 'not found', occurrId: 0);
+    final correspondingCourseUnit = courseUnits.firstWhere(
+      (courseUnit) => courseUnit.occurrId == occurrId,
+      orElse: () => notFound,
+    );



Isn't there a way to pass down the courseUnit from the parent that build the schedule slot?

Try to avoid sentinel values. Return null in case of error.

uni/lib/view/schedule/widgets/schedule_slot.dart

limwa · 2024-02-23T13:13:56Z

uni/lib/view/schedule/widgets/schedule_slot.dart

+    if (correspondingCourseUnit(context).name == 'not found') {
+      return const Column(


If you can't find a way to pass down the course unit from the parent of the schedule slot, you could run this in the initState method so that this doesn't every time the component is rebuilt!

uni/lib/view/schedule/widgets/schedule_slot.dart

bdmendes · 2024-02-28T20:34:25Z

uni/lib/view/schedule/widgets/schedule_slot.dart

+    final courseUnits =
+        Provider.of<ProfileProvider>(context, listen: false).state!.courseUnits;
+    final notFound =
+        CourseUnit(abbreviation: 'NF', name: 'not found', occurrId: 0);
+    final correspondingCourseUnit = courseUnits.firstWhere(
+      (courseUnit) => courseUnit.occurrId == occurrId,
+      orElse: () => notFound,
+    );



Try to avoid sentinel values. Return null in case of error.

Co-authored-by: Adriana <[email protected]>

limwa

LGTM 🚀

adriannalmeida requested a review from bdmendes November 17, 2023 15:48

bdmendes requested changes Nov 17, 2023

View reviewed changes

uni/lib/view/schedule/widgets/schedule_slot.dart Outdated Show resolved Hide resolved

uni/lib/view/schedule/widgets/schedule_slot.dart Outdated Show resolved Hide resolved

adriannalmeida self-assigned this Dec 5, 2023

adriannalmeida force-pushed the feature/uc-button branch 2 times, most recently from be3ea97 to ca7a80a Compare December 5, 2023 00:32

bdmendes requested changes Dec 5, 2023

View reviewed changes

uni/lib/generated/intl/messages_all.dart Outdated Show resolved Hide resolved

uni/lib/generated/intl/messages_en.dart Show resolved Hide resolved

uni/lib/view/schedule/widgets/schedule_slot.dart Outdated Show resolved Hide resolved

bdmendes force-pushed the feature/uc-button branch from ca7a80a to b1d342d Compare December 5, 2023 13:08

bdmendes requested a review from vitormpp December 8, 2023 14:32

adriannalmeida force-pushed the feature/uc-button branch 5 times, most recently from d058b08 to 9f45374 Compare December 22, 2023 18:12

bdmendes approved these changes Dec 22, 2023

View reviewed changes

DGoiana reviewed Jan 22, 2024

View reviewed changes

uni/lib/generated/intl/messages_en.dart Show resolved Hide resolved

thePeras requested changes Feb 7, 2024

View reviewed changes

uni/lib/view/schedule/widgets/schedule_slot.dart Outdated Show resolved Hide resolved

uni/lib/view/schedule/widgets/schedule_slot.dart Outdated Show resolved Hide resolved

bdmendes requested changes Feb 14, 2024

View reviewed changes

niaefeup-admin force-pushed the develop branch from 417c250 to bd61b50 Compare February 20, 2024 16:55

limwa force-pushed the develop branch from bd61b50 to 417c250 Compare February 20, 2024 17:29

bdmendes added this to the March 2023 Release milestone Feb 23, 2024

limwa requested changes Feb 23, 2024

View reviewed changes

thePeras self-requested a review February 24, 2024 16:19

limwa force-pushed the feature/uc-button branch from 568d250 to 1a6983b Compare February 28, 2024 15:12

limwa requested changes Feb 28, 2024

View reviewed changes

uni/lib/view/schedule/widgets/schedule_slot.dart Outdated Show resolved Hide resolved

limwa force-pushed the feature/uc-button branch from 1a6983b to 519da09 Compare February 28, 2024 15:46

bdmendes requested changes Feb 28, 2024

View reviewed changes

adriannalmeida force-pushed the feature/uc-button branch from e62adcc to 8c9a9c3 Compare February 29, 2024 12:35

adriannalmeida added 2 commits February 29, 2024 12:36

change uc link

2e8c72c

changed

71adc76

adriannalmeida and others added 6 commits February 29, 2024 12:36

formated everything

a245a4d

corrected the translations

dea952f

Handling non registered ucs

8662e00

Fix unit tests

652c800

Co-authored-by: Adriana <[email protected]>

small changes

182c021

Extract corresponding course unit to schedule slot

2d0c6c2

adriannalmeida force-pushed the feature/uc-button branch from 8c9a9c3 to 2d0c6c2 Compare February 29, 2024 12:36

bdmendes approved these changes Feb 29, 2024

View reviewed changes

limwa approved these changes Feb 29, 2024

View reviewed changes

bdmendes merged commit fc08b02 into develop Feb 29, 2024
6 checks passed

bdmendes deleted the feature/uc-button branch February 29, 2024 13:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changing uc link to open in app page #1035

Changing uc link to open in app page #1035

adriannalmeida commented Nov 17, 2023

codecov bot commented Nov 17, 2023 •

edited

Loading

bdmendes left a comment

bdmendes left a comment •

edited

Loading

bdmendes left a comment

bdmendes left a comment

limwa left a comment

limwa Feb 23, 2024

bdmendes Feb 28, 2024

limwa Feb 23, 2024

bdmendes Feb 28, 2024

limwa left a comment

		if (correspondingCourseUnit(context).name == 'not found') {
		return const Column(

Changing uc link to open in app page #1035

Changing uc link to open in app page #1035

Conversation

adriannalmeida commented Nov 17, 2023

Review checklist

codecov bot commented Nov 17, 2023 • edited Loading

Codecov Report

bdmendes left a comment

Choose a reason for hiding this comment

bdmendes left a comment • edited Loading

Choose a reason for hiding this comment

bdmendes left a comment

Choose a reason for hiding this comment

bdmendes left a comment

Choose a reason for hiding this comment

limwa left a comment

Choose a reason for hiding this comment

limwa Feb 23, 2024

Choose a reason for hiding this comment

bdmendes Feb 28, 2024

Choose a reason for hiding this comment

limwa Feb 23, 2024

Choose a reason for hiding this comment

bdmendes Feb 28, 2024

Choose a reason for hiding this comment

limwa left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 17, 2023 •

edited

Loading

bdmendes left a comment •

edited

Loading