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.

Sign up for GitHub

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

Jump to bottom

Feature 7 course enrolment debugging #83

Merged
merged 7 commits into from
Dec 8, 2022
Merged

Feature 7 course enrolment debugging #83

merged 7 commits into from
Dec 8, 2022

Conversation

jltng
Copy link
Contributor

@jltng jltng commented Dec 6, 2022 •
edited
Loading

As of December 7th, 2022:
MAJOR CHANGES TO COURSE ENROLMENT USE CASE

  • created 3 gateways, respectively implemented by FileCourse, FileUser, and FileTaskMap (added necessary methods in FileUser and FileTaskMap @CC-3636 @yyutiw
  • this being said, changed gateway names @TiareMar changed name in your use case + tests to code would run. But I don't think you can directly call my gateway in your use case (you would need your own use case gateway to implement FileCourse)
  • removed all occurrences of casting
  • enrolment screen should work (by work i mean the success view pop up lol) !!!
  • @celinelumqr I commented out one of your assertion tests because it failed for me:

Screen Shot 2022-12-07 at 6 34 46 PM

  • finish enrolment interactor test
  • do some more testing
  • report my use cases

As of December 5th, 2022
Changes in CourseEnrolmentInteractor:

  • Need to pull main branch (gave me a scary error about a .git/ file that was changed????
  • As the instructor enters "task1,task2,task3" but we want ["task1", "task2", "task3"] added a method in my CourseCreationRequest model that splits the input string by comma and appends it to an ArrayList. Courses.getTasks() should produce ["task1", "task2", "task3"].
  • Small bug: student's ID is never added to the 'students' parameter of the Course (the second check for course enrolment would always pass because there are no students in the Course)
  • Bug: EnrolmentInteractor throwing a NPE: parameters in response model are set to null (not initialized properly)
    Screen Shot 2022-12-06 at 11 02 00 PM
  • Need to change the way the tasks datafile is accessed --> FileTaskMap needs to implement my use case gateway
  • Have a few tests here because I need my new edits for them to work. Will use testing branch when this gets merged.

@jltng
changed logic of making of a copy of a map of key-value pairs and cha…
35aaee6 
…nging the key so that it no longer requires casting (uses parallel arraylists), added helper method in TaskMap that allows me to add a Map to TaskMap
@jltng jltng linked an issue Dec 6, 2022 that may be closed by this pull request
Feature 7 - Course Enrolment and Course Creation #16
Open
4 tasks
@jltng jltng self-assigned this Dec 6, 2022
@jltng jltng requested a review from TiareMar December 6, 2022 20:25
@jltng jltng added the (de)bug (Fixing) something that isn't working label Dec 6, 2022
TiareMar
TiareMar reviewed Dec 6, 2022
View reviewed changes
...main/java/use_cases/course_features/course_enrolment_use_case/CourseEnrolmentInteractor.java Outdated
@@ -84,39 +85,44 @@ public CourseEnrolmentResponseModel enrol(CourseEnrolmentRequestModel requestMod
courseTaskId.add(taskTitleToId);
Copy link
Contributor

Choose a reason for hiding this comment

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

above on line 46/47, I think it should be if (!courseEnrolmentDsGateway.existsByCourseID(requestModel.getCourseID())) { (i.e. if course does NOT exist)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed! Thanks for catching it

@TiareMar
Copy link
Contributor

TiareMar commented Dec 6, 2022

Also, while you're making changes, I think the screen issue for the courses tab in the main screens are just that in the ActionListeners of StudentMain and InstructorMain need to be showing the screen name that you initialize in Main. (i.e. 'courseEnrol' instead of just 'course')

jltng added 6 commits December 6, 2022 23:08
@jltng
npe still not solved in interactor :,( but added necessary functions …
9a576ab 
…in course creation use case (splitting one string of tasks into multiple task strings in arraylist in requestmodel, made pop-up text nicer in screen, modified Course entity getTasks method to match use case changes, one small bug found,
@jltng
pulling changes from main
cfcdcc2
@jltng
started course creation tests (cannot get presenter not controller to…
d567611 
… work rip), still need to test for persistence as well as the enrolment tests too
@jltng
MAJOR CHANGES TO ENROLMENT USE CASE (uncessary files commented out, w…
6deb23d 
…ill delete later after i make sure this actually works)
@jltng
edited interactor name in tiare's use case + tests
8f687cd
@jltng
fixing autograding error
d80bb81
alyson647
alyson647 approved these changes Dec 7, 2022
View reviewed changes
Copy link
Contributor

@alyson647 alyson647 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

TiareMar
TiareMar approved these changes Dec 8, 2022
View reviewed changes
Copy link
Contributor

@TiareMar TiareMar left a comment

Choose a reason for hiding this comment

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

Looking good!

CC-3636
CC-3636 approved these changes Dec 8, 2022
View reviewed changes
Copy link
Contributor

@CC-3636 CC-3636 left a comment

Choose a reason for hiding this comment

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

looks swaggy to me

@jltng jltng merged commit 404b8ee into main Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alyson647 alyson647 alyson647 approved these changes

@CC-3636 CC-3636 CC-3636 approved these changes

@TiareMar TiareMar TiareMar approved these changes

@celinelumqr celinelumqr Awaiting requested review from celinelumqr

@yyutiw yyutiw Awaiting requested review from yyutiw

@sshmuylovich sshmuylovich Awaiting requested review from sshmuylovich

Assignees

@jltng jltng

Labels
(de)bug (Fixing) something that isn't working
Projects
None yet
Milestone
Debugging, Fully Functional Program
Development

Successfully merging this pull request may close these issues.

Feature 7 - Course Enrolment and Course Creation
4 participants
@jltng @TiareMar @CC-3636 @alyson647