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

fix: allow user to request new seasons of a already provided show #966

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open

fix: allow user to request new seasons of a already provided show #966

wants to merge 8 commits into from

Conversation

ToMattBan
Copy link

Description

Follow the flow:

  • User "Bob" request "Vikings"
  • I complete the request by adding "Vikings" in the server
  • After 2 months, a new Vikings season is out
  • User "John" try to request the new season
  • No "Request" button is available

This PR aims to fix this, by adding a "Request More" button that allows the user to request a new season of the show.

Screenshot (if UI-related)

image

Issues Fixed or Closed

@ToMattBan ToMattBan mentioned this pull request Sep 13, 2024
Closed
1 task
Gauvino
Gauvino reviewed Sep 13, 2024
View reviewed changes
src/i18n/locale/pt_BR.json Outdated Show resolved Hide resolved
@ToMattBan
feat(component): allow users to request new seasons
43d204c 
When a new season of a show already available launches, the users can't ask for the new season as
the request more button doesn't appear, this commit aims to fix this

fallenbagel#962
@gauthier-th gauthier-th changed the title Allow user to request new seasons of a already provided show fix: allow user to request new seasons of a already provided show Sep 15, 2024
gauthier-th
gauthier-th requested changes Nov 11, 2024
View reviewed changes
src/components/RequestButton/index.tsx Outdated
Comment on lines 295 to 302
} else if (
mediaType === 'tv' &&
hasPermission([Permission.REQUEST, Permission.REQUEST_TV], {
type: 'or',
}) &&
media &&
media.status === MediaStatus.PARTIALLY_AVAILABLE
) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should merge this condition with the one below, both are doing the same thing

Copy link
Owner

Choose a reason for hiding this comment

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

@ToMattBan could you make this change asap so we can have it available for next version?

Copy link
Author

Choose a reason for hiding this comment

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

Done, sorry the delay

@fallenbagel
Copy link
Owner

@ToMattBan You just removed the fix you wanted this PR to achieve and only white-space changes remaining now

@ToMattBan
Copy link
Author

@fallenbagel yeah, I'm not sure what you want. It was said that I should merge with the one bellow, when I did, the changes don't exist anymore. If they do the same thing, there are no reason to my fix.

@fallenbagel
Copy link
Owner

@fallenbagel yeah, I'm not sure what you want. It was said that I should merge with the one bellow, when I did, the changes don't exist anymore. If they do the same thing, there are no reason to my fix.

We literally asked to merge your condition PENDING_APPROVAL permission into the logic below...

We still need this fix but you'll need to add the permision to the logic below instead of the else condition. That's all we said...

fallenbagel
fallenbagel requested changes Dec 30, 2024
View reviewed changes
Copy link
Owner

@fallenbagel fallenbagel left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Line 303 achieves the same logic as the else function that this pr originally had. We asked to merge that else function into line 303

You just need to add PENDING_APPROVAL into the else if conditions. Same with 4k button

@ToMattBan
Copy link
Author

Gotcha, as soon as I get home, I fix it.

@ToMattBan
Copy link
Author

@fallenbagel done. Tested a lot and it seems stable. No problem found.
I also cleaned where the request button is called, removing the unused code.

The reason to remove isComplete is that media.status !== MediaStatus.AVAILABLE kinda of does the trick, as "Available" is the shows/movies that are FULLY available, if not, the status is PARTIALLY_AVAILABLE

@github-actions github-actions bot added the merge conflict Cannot merge due to merge conflicts label Dec 31, 2024
@github-actions GitHub Actions
Copy link

This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged.

@github-actions github-actions bot removed the merge conflict Cannot merge due to merge conflicts label Dec 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Gauvino Gauvino Gauvino left review comments

@fallenbagel fallenbagel fallenbagel requested changes

@gauthier-th gauthier-th gauthier-th requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

series: new seasons not requestable if older seasons already here
4 participants
@ToMattBan @fallenbagel @gauthier-th @Gauvino