Skip to content

✨Issue-196 course user wishlist models and endpoints#207

Merged
sampaccoud merged 2 commits into
mainfrom
issue-196_morgane_course_user_wishlist
Jul 7, 2023
Merged

✨Issue-196 course user wishlist models and endpoints#207
sampaccoud merged 2 commits into
mainfrom
issue-196_morgane_course_user_wishlist

Conversation

@MorganeAD
Copy link
Copy Markdown
Contributor

@MorganeAD MorganeAD commented Feb 9, 2023 •
edited by sampaccoud
Loading

This PR is the first part of the issue #196 (feature user course wishlist)

This PR adds:

  • model CourseWish that link a User and a Course
  • action endpoint on the course API to handle this new model, according to the authenticated user
GET /api/courses/:<course_id|course_code>/wish
    Return wish status on this course for the authenticated user

POST /api/courses/:<course_id|course_code>/wish
    Confirm a wish on this course for the authenticated user

DELETE /api/courses/:<course_id|course_code>/wish
    Delete any existing wish on this course for the authenticated user

@MorganeAD MorganeAD force-pushed the issue-196_morgane_course_user_wishlist branch 2 times, most recently from 5dee84b to a5cc3a5 Compare February 9, 2023 16:46
sampaccoud
sampaccoud reviewed Feb 10, 2023
View reviewed changes
Comment thread src/backend/joanie/core/models/wishlist.py Outdated
@MorganeAD MorganeAD force-pushed the issue-196_morgane_course_user_wishlist branch 2 times, most recently from b9ad30d to e2e9b46 Compare February 16, 2023 07:33
@MorganeAD MorganeAD force-pushed the issue-196_morgane_course_user_wishlist branch 3 times, most recently from 2922716 to ef63832 Compare February 23, 2023 07:51
@MorganeAD
Copy link
Copy Markdown
Contributor Author

MorganeAD commented Feb 23, 2023

According to our discussion with @sampaccoud and @rlecellier , I renamed the model "WishList" to "CourseWish" because:

  • it's specify it's a wish related of an Course object
  • Wish and not WishList because it represents one wish of a list of wishes

@MorganeAD
Copy link
Copy Markdown
Contributor Author

MorganeAD commented Feb 23, 2023

Those routes has been tested locally with the front in Richie of this PR openfun/richie#1904 and it works

@MorganeAD MorganeAD changed the title ✨ Issue-196 course user wishlist models and endpoints ✨Issue-196 course user wishlist models and endpoints Feb 23, 2023
@MorganeAD MorganeAD force-pushed the issue-196_morgane_course_user_wishlist branch from ef63832 to 1aa2177 Compare February 28, 2023 15:04
@MorganeAD MorganeAD mentioned this pull request Feb 28, 2023
Open
@MorganeAD MorganeAD force-pushed the issue-196_morgane_course_user_wishlist branch from 1aa2177 to d280c9d Compare February 28, 2023 16:08
carofun
carofun reviewed Feb 28, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@carofun carofun left a comment

Choose a reason for hiding this comment

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

👍

a small return in the commit message

that link => that links

Don't you think we will need a field to inactivate the wish?

Comment thread src/backend/joanie/tests/core/test_api_wishlist.py Outdated
Comment thread src/backend/joanie/tests/core/test_api_wishlist.py Outdated
Comment thread src/backend/joanie/core/api.py Outdated
@MorganeAD MorganeAD force-pushed the issue-196_morgane_course_user_wishlist branch 2 times, most recently from f9b6bc4 to 06f3ee7 Compare March 1, 2023 08:09
@MorganeAD
Copy link
Copy Markdown
Contributor Author

MorganeAD commented Mar 1, 2023

thanks @carofun for your review, here your requested changes 06f3ee7

jbpenrath
jbpenrath reviewed Mar 2, 2023
View reviewed changes
Comment thread CHANGELOG.md Outdated
Comment thread src/backend/joanie/core/models/wishlist.py Outdated
Comment thread src/backend/joanie/core/models/wishlist.py Outdated
Comment thread src/backend/joanie/core/models/__init__.py Outdated
Comment thread src/backend/joanie/core/models/wishlist.py
Comment thread src/backend/joanie/tests/core/test_api_wishlist.py Outdated
Comment thread src/backend/joanie/core/api.py Outdated
Comment thread src/backend/joanie/core/api.py Outdated
@MorganeAD MorganeAD force-pushed the issue-196_morgane_course_user_wishlist branch 3 times, most recently from 6417199 to 88c26bc Compare March 7, 2023 10:52
@sampaccoud sampaccoud force-pushed the issue-196_morgane_course_user_wishlist branch from 88c26bc to 531830b Compare July 4, 2023 22:24
@sampaccoud sampaccoud requested a review from jbpenrath July 4, 2023 22:25
@sampaccoud sampaccoud assigned sampaccoud and unassigned MorganeAD Jul 4, 2023
@sampaccoud sampaccoud force-pushed the issue-196_morgane_course_user_wishlist branch from 531830b to 455a5f9 Compare July 4, 2023 22:36
jbpenrath
jbpenrath reviewed Jul 6, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@jbpenrath jbpenrath left a comment

Choose a reason for hiding this comment

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

Are we agree that a wish means I want to be warned when :

  1. A new course run / products is added to the course
  2. A previously full product has a vacancy

?

Comment thread src/backend/joanie/core/api/client.py Outdated
@sampaccoud sampaccoud requested a review from jbpenrath July 7, 2023 13:18
jbpenrath
jbpenrath approved these changes Jul 7, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@jbpenrath jbpenrath left a comment

Choose a reason for hiding this comment

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

LGTM ✅

Comment thread src/backend/joanie/tests/core/test_api_course.py Outdated
@sampaccoud sampaccoud force-pushed the issue-196_morgane_course_user_wishlist branch from 6f34e11 to d5363a3 Compare July 7, 2023 16:33
@sampaccoud
✨(models/api) add models and endpoints to user wishlist
71a58e4 
This commit is the first part of resolving issue 196 (course
wishes). We add a CourseWish model and an API endpoint as
action on the existing course endpoint.

co-authored with Morgane Alonso <malonso@hybird.org>
@sampaccoud sampaccoud force-pushed the issue-196_morgane_course_user_wishlist branch from d5363a3 to 6c7da60 Compare July 7, 2023 16:34
@sampaccoud sampaccoud enabled auto-merge (rebase) July 7, 2023 16:35
@sampaccoud
✨(api) allow retrieving courses by their code as well as their pk
06b0584 
Richie knows the code but not the ID of courses.
@sampaccoud sampaccoud force-pushed the issue-196_morgane_course_user_wishlist branch from 6c7da60 to 06b0584 Compare July 7, 2023 16:48
@sampaccoud sampaccoud merged commit 933aa09 into main Jul 7, 2023
@sampaccoud sampaccoud deleted the issue-196_morgane_course_user_wishlist branch July 7, 2023 16:57
@NathanVss
Copy link
Copy Markdown
Collaborator

NathanVss commented Jul 11, 2023

Even if the course code should work it appears to not work on my side, please see the different errors:

Capture d’écran 2023-07-11 à 16 44 52 Capture d’écran 2023-07-11 à 16 45 25

The course actually exists on the admin and in the db:
Capture d’écran 2023-07-11 à 16 45 37
Capture d’écran 2023-07-11 à 16 45 59

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

Reviewers

@jbpenrath jbpenrath jbpenrath approved these changes

+2 more reviewers

@sampaccoud sampaccoud sampaccoud left review comments

@carofun carofun carofun left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@sampaccoud sampaccoud

Labels

feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@MorganeAD @NathanVss @sampaccoud @jbpenrath @carofun