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

fix: if ticket is not available for sale ,don't show event #7443

Closed
wants to merge 2 commits into from
Closed

fix: if ticket is not available for sale ,don't show event #7443

wants to merge 2 commits into from

Conversation

maze-runnar
Copy link
Contributor

Fixes fossasia/open-event-frontend#5110

Checklist

  • I have read the Contribution & Best practices Guide and my PR follows them.
  • My branch is up-to-date with the Upstream development branch.
  • The unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • All the functions created/modified in this PR contain relevant docstrings.

@auto-label auto-label bot added the fix label Nov 13, 2020
@codecov
Copy link

codecov bot commented Nov 13, 2020

Codecov Report

Merging #7443 (41ad027) into development (ea8a477) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #7443      +/-   ##
===============================================
- Coverage        65.28%   65.27%   -0.01%     
===============================================
  Files              265      265              
  Lines            13223    13224       +1     
===============================================
  Hits              8632     8632              
- Misses            4591     4592       +1     
Impacted Files Coverage Δ
app/api/events.py 38.82% <0.00%> (-0.10%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea8a477...41ad027. Read the comment docs.

@@ -858,7 +858,7 @@ def query(self, view_kwargs):
Event.event_type_id != None,
Event.event_topic_id != None,
Event.event_sub_topic_id != None,
Event.tickets.any(and_(Ticket.deleted_at == None, Ticket.is_hidden == False, Ticket.sales_ends_at > current_time)),
Event.tickets.any(and_(Ticket.deleted_at == None, Ticket.is_hidden == False, Ticket.sales_ends_at > current_time, Ticket.is_available == True)),
Copy link
Member

Choose a reason for hiding this comment

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

is_available is not a column. It's a computed property. It doesn't work like this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the query that is written in is_available property is that right for the condition?can that query be written here again for this work?

Copy link
Member

Choose a reason for hiding this comment

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

Yes

Copy link
Member

Choose a reason for hiding this comment

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

But please run and confirm before committing. Just visit /v1/events/upcoming to verify

@@ -858,7 +859,7 @@ def query(self, view_kwargs):
Event.event_type_id != None,
Event.event_topic_id != None,
Event.event_sub_topic_id != None,
Event.tickets.any(and_(Ticket.deleted_at == None, Ticket.is_hidden == False, Ticket.sales_ends_at > current_time, Ticket.is_available == True)),
Event.tickets.any(and_(Ticket.deleted_at == None, Ticket.is_hidden == False, Ticket.sales_ends_at > current_time, Ticket.quantity > get_sold_and_reserved_tickets_count(Ticket.id))),
Copy link
Member

Choose a reason for hiding this comment

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

Won't work. This is not a python loop, only allowed operations here are sqlalchemy scalar and join type operations

Copy link
Member

Choose a reason for hiding this comment

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

Please verify by running before submitting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's working, I have checked it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have an event, it has two tickets and 1 ticket of each type is sold.then after applying above I edited event and changes ticket number to 1 of both events then the event is not on the front page anymore. Then i changed ticket numbers for one ticket to one and for another ticket to 100. Then event is showing on front page.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, yes, it'll work indeed. But I'll have to check if the query is efficient. Sorry for not noticing Ticket.id instead of ticket.id

Copy link
Member

Choose a reason for hiding this comment

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

Doesn't work, it statically returns all reserved and sold tickets and compares ticket quantity with it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

get_sold_and_reserved_tickets_count(Ticket.id) what is meaning of passing ticked.id here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

all reserved and sold tickets for a ticket ?

Copy link
Member

Choose a reason for hiding this comment

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

It won't work because it expects a ticket ID. Passing Ticket.id just returns all tickets sold count

@maze-runnar maze-runnar deleted the patch-6 branch November 26, 2020 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Frontpage: Limit display of events based on the public ticket availability
2 participants