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

#197 if event began already and has only offline payment method then … #262

Merged

cbellone merged 2 commits into alfio-event:master from Praitheesh:197_after_event_start_error

Apr 8, 2017

Contributor

Praitheesh commented Feb 17, 2017

if event began already and has only offline payment method then display warning message at event show-event page and disable forward button to reservation/payment page.
if even began already and has multiple payment method including offline payment then not show offline payment as option in reservation page


          alfio-event#197 if event began already and has only offline payment m…

bcb60a3

…ethod then display warning message at event show-event page and disable forward button to reservation/payment page. if even began already and has multiple payment method including offline payment then not show offline payment as option in reservation page

Member

syjer commented Feb 17, 2017

Thank you @Praitheesh , I'll have a look 👍

coveralls commented Feb 17, 2017

Coverage decreased (-0.007%) to 53.903% when pulling bcb60a3 on Praitheesh:197_after_event_start_error into 72d8e0e on exteso:master.

4 similar comments

coveralls commented Feb 17, 2017

Coverage decreased (-0.007%) to 53.903% when pulling bcb60a3 on Praitheesh:197_after_event_start_error into 72d8e0e on exteso:master.

coveralls commented Feb 17, 2017 •

edited

Coverage decreased (-0.007%) to 53.903% when pulling bcb60a3 on Praitheesh:197_after_event_start_error into 72d8e0e on exteso:master.

coveralls commented Feb 17, 2017

Coverage decreased (-0.007%) to 53.903% when pulling bcb60a3 on Praitheesh:197_after_event_start_error into 72d8e0e on exteso:master.

coveralls commented Feb 17, 2017

Coverage decreased (-0.007%) to 53.903% when pulling bcb60a3 on Praitheesh:197_after_event_start_error into 72d8e0e on exteso:master.

Contributor Author

Praitheesh commented Mar 15, 2017

@syjer have you got chance to look into this PR ?

Member

syjer commented Mar 15, 2017

Hi @Praitheesh , sorry for taking so much time, we were quite busy. Globally it seems ok, I'll try to merge it this week 👍

cbellone reviewed

View reviewed changes

Member

cbellone left a comment

Thank you for submitting this PR!

src/main/java/alfio/controller/ReservationController.java Outdated

                                   OrderSummary orderSummary = ticketReservationManager.orderSummaryForReservationId(reservationId, event, locale);
                                   List<PaymentProxy> activePaymentMethods = paymentManager.getPaymentMethods(event.getOrganizationId())
                                       .stream()
-                                      .filter(p -> p.isActive() && event.getAllowedPaymentProxies().contains(p.getPaymentProxy()))
+                                      .filter(p -> p.isActive() && event.getAllowedPaymentProxies().contains(p.getPaymentProxy()) && !p.getPaymentProxy().equals(PaymentProxy.OFFLINE) || (p.getPaymentProxy().equals(PaymentProxy.OFFLINE) && p.isActive() && event.getAllowedPaymentProxies().contains(p.getPaymentProxy()) && TicketReservationManager.hasValidOfflinePaymentWaitingPeriod(event, configurationManager)))

Member

cbellone Apr 6, 2017

Could we simplify this condition?
I was thinking something like:

p -> p.isActive() && event.getAllowedPaymentProxies().contains(p.getPaymentProxy()) && (!p.getPaymentProxy().equals(PaymentProxy.OFFLINE) || TicketReservationManager.hasValidOfflinePaymentWaitingPeriod(event, configurationManager)))

would that work?

In any case I think we could extract this filter in a method, so that the code would be more readable. WDYT?

Contributor Author

Praitheesh Apr 8, 2017

Yes agree.

src/main/resources/alfio/i18n/public.properties Outdated

               show-event.sold-out.subscribe=subscribe
               show-event.sold-out.subscription-complete=Thank you for joining the waiting queue. We''ll keep you posted!
               show-event.sold-out.subscription-error=Something went wrong, please retry in few minutes. Thank you!
+              show-event.offline-payment-not-available=We''re sorry, Cannot confirm an offline reservation after event start.

Member

cbellone Apr 6, 2017

I would change this to a more generic: "Due to a wrong configuration, it''s not possible to sell tickets right now. Please contact the organizers." or something like that.

Could you please add the placeholders also for other languages?
E.g. in public_it.properties:
show-event.offline-payment-not-available=IT-Due to a wrong configuration, it''s not possible to sell tickets right now. Please contact the organizers.

Contributor Author

Praitheesh Apr 8, 2017

done.

src/main/java/alfio/controller/EventController.java Outdated

                           List<SaleableTicketCategory> validCategories = ticketCategories.stream().filter(tc -> !tc.getExpired()).collect(Collectors.toList());
                           List<SaleableAdditionalService> additionalServices = additionalServiceRepository.loadAllForEvent(event.getId()).stream().map((as) -> getSaleableAdditionalService(event, locale, as, promoCodeDiscount.orElse(null))).collect(Collectors.toList());
                           Predicate<SaleableTicketCategory> waitingQueueTargetCategory = tc -> !tc.getExpired() && !tc.isBounded();
+                          boolean inValidOfflinePayment = event.getAllowedPaymentProxies().size() == 1 && event.getAllowedPaymentProxies().contains(PaymentProxy.OFFLINE) && !TicketReservationManager.hasValidOfflinePaymentWaitingPeriod(event, configurationManager);

Member

cbellone Apr 6, 2017

could we maybe generalize this so that it could disable the button and show the warning also if there is no payment proxy available, but only if the event is not free of charge?

Contributor Author

Praitheesh Apr 8, 2017

Good point. I'll add additional check. Thanks.


          alfio-event#197 validate payment methods exsit for non-free events , …

f5145ca

…update warning message

coveralls commented Apr 8, 2017 •

edited

Coverage increased (+0.07%) to 53.981% when pulling f5145ca on Praitheesh:197_after_event_start_error into 72d8e0e on exteso:master.

3 similar comments

coveralls commented Apr 8, 2017

Coverage increased (+0.07%) to 53.981% when pulling f5145ca on Praitheesh:197_after_event_start_error into 72d8e0e on exteso:master.

coveralls commented Apr 8, 2017

Coverage increased (+0.07%) to 53.981% when pulling f5145ca on Praitheesh:197_after_event_start_error into 72d8e0e on exteso:master.

coveralls commented Apr 8, 2017

Coverage increased (+0.07%) to 53.981% when pulling f5145ca on Praitheesh:197_after_event_start_error into 72d8e0e on exteso:master.

cbellone merged commit a7708dd into alfio-event:master

Member

cbellone commented Apr 8, 2017

merged. Thank you! 👍

Praitheesh deleted the 197_after_event_start_error branch

April 8, 2017 09:00

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