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.

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

Allowing unsetting of favourite program #5151

Merged
merged 33 commits into from
Apr 9, 2024
Merged

Allowing unsetting of favourite program #5151

merged 33 commits into from
Apr 9, 2024

Conversation

TiBiBa
Copy link
Collaborator

@TiBiBa TiBiBa commented Feb 14, 2024

Fixes #5132

How to test:

  1. Login (have a public profile and a public program)
  2. Click on favourite a program and unfavourite the program
Screen.Recording.2024-03-27.at.10.21.31.mov

@TiBiBa TiBiBa marked this pull request as draft February 14, 2024 13:24
@ghost
Copy link

ghost commented Feb 14, 2024

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@Felienne
Copy link
Member

Hi @hasan-sh!

This is a somewhat longstanding R, could you pick it up? I am not sure how ready it is but I think it can be a small PR in between other things.

@hasan-sh
Copy link
Collaborator

hasan-sh commented Mar 19, 2024

Hi @hasan-sh!

This is a somewhat longstanding R, could you pick it up? I am not sure how ready it is but I think it can be a small PR in between other things.

Will do so after the autosave PR and finish investigating the login problems that we discussed with the teachers!

@Annelein Annelein marked this pull request as ready for review March 21, 2024 12:50
@Annelein Annelein requested a review from hasan-sh March 21, 2024 12:50
@Annelein Annelein removed the request for review from hasan-sh March 21, 2024 13:05
@Annelein Annelein requested a review from hasan-sh March 21, 2024 13:28
@Annelein Annelein removed their assignment Mar 27, 2024
Copy link
Collaborator

@hasan-sh hasan-sh left a comment

Choose a reason for hiding this comment

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

I cannot favourite a program without a refresh. I'll review the code once this is done! Let me know if you need any help:)

@Annelein Annelein requested a review from hasan-sh April 1, 2024 08:42
@Annelein
Copy link
Collaborator

Annelein commented Apr 3, 2024

@hasan-sh I've also added a test for favouriting and unfavouriting a program as mentioned in #4251, can you let me know if this is how a test is supposed to be?

@hasan-sh
Copy link
Collaborator

hasan-sh commented Apr 3, 2024

Great @Annelein , will review your PR

@Annelein
Copy link
Collaborator

Annelein commented Apr 3, 2024

Great @Annelein , will review your PR

Thank you! Then I can continue working on #4251 if I know that this is the correct test approach

Copy link
Collaborator

@hasan-sh hasan-sh left a comment

Choose a reason for hiding this comment

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

Hi, i remember making it possible to update the UI from TS; ffa813a#diff-810bc5b30fefd1c7d50d825f1331efb7cda58a958fdbe93d015904aed1735a47R29-R35

Why did you move the data-starred attribute to the i element instead of the container? Now you the ui isn't updating as it was!

I suggested a change that fixes the issue, but do take a look at the test and whether anything else depends on what i changed!

templates/htmx-program.html Outdated Show resolved Hide resolved
@Annelein
Copy link
Collaborator

Annelein commented Apr 8, 2024

Hi, i remember making it possible to update the UI from TS; ffa813a#diff-810bc5b30fefd1c7d50d825f1331efb7cda58a958fdbe93d015904aed1735a47R29-R35

Why did you move the data-starred attribute to the i element instead of the container? Now you the ui isn't updating as it was!

I did this because the test was failing because it would miss click because this is the space the container takes up, so now the test fails again:
Screenshot 2024-04-08 at 09 31 38

Pushed another fix.

@Annelein Annelein requested a review from hasan-sh April 8, 2024 07:41
@hasan-sh
Copy link
Collaborator

hasan-sh commented Apr 8, 2024

@Annelein the set_favourite_program uses need to be fixed. Try to make a program private after favouriting it.

Copy link
Contributor

mergify bot commented Apr 9, 2024

Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit d1ba0e8 into main Apr 9, 2024
12 checks passed
@mergify mergify bot deleted the unfavourite-program branch April 9, 2024 14:39
Copy link
Contributor

mergify bot commented Apr 9, 2024

Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

💻 Cannot unfavourite a program
5 participants