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

Send mail on ticket confirmation with attached ticket pdf. #1600

Merged
merged 1 commit into from Jul 26, 2017

Conversation

siddhantbajaj
Copy link
Contributor

No description provided.

Copy link
Member

@Ana06 Ana06 left a comment

Choose a reason for hiding this comment

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

and please add a description to your commit message 😉

@@ -0,0 +1,7 @@
class AddTicketConfirmationToEmailSettings < ActiveRecord::Migration
def change
add_column :email_settings, :send_on_ticket_confirmation, :boolean, default: false
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need all that?

I mean. I would say this email should always be sent. And I think we don't need to customize the message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And I think we don't need to customize the message.

So should we set the message manually or like use the same message that is sent in registration confirmation email?

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 would say this email should always be sent.

👍

Copy link
Member

Choose a reason for hiding this comment

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

You can send a different email, but the subject can always be something like Your tickets for ConferenceName. So I mean that you can find a default message that can be used for all conferences, so the admin doesn't need to configure this. It is also less confusing for user, who will always receive similar email.

And in this way, you don't need to store anything in the DB. 😉


def send_ticket_confirmation_mail
if conference.email_settings.send_on_ticket_confirmation?
Mailbot.ticket_confirmation_mail(conference, user, self).deliver_later
Copy link
Member

Choose a reason for hiding this comment

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

We were talking about not sending two emails, as when paying the tickets there is an email already sent, what is the state of that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No unfortunately we cannot send the ticket as a attachment in stripe confirmation email. Customisation of stripe confirmation email is only limited to changing the organisation name, logo.

Copy link
Member

Choose a reason for hiding this comment

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

so we will always send two emails in this case, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes one for payment confirmation and the the other one for ticket confirmation.

@siddhantbajaj siddhantbajaj force-pushed the pdf-attachment branch 3 times, most recently from bfa4aed to 8241670 Compare July 25, 2017 13:33
@@ -0,0 +1,8 @@
Dear <%= @user.name %>,

Thanks! You have successfully booked a <%= @physical_ticket.ticket.title %> ticket for the event <%= @conference.title %>.
Copy link
Member

Choose a reason for hiding this comment

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

I would also add the token in the email

@@ -6,6 +6,7 @@ class PhysicalTicket < ActiveRecord::Base
has_many :ticket_scannings

before_create :set_token
after_create :send_ticket_confirmation_mail
Copy link
Member

Choose a reason for hiding this comment

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

there is something I don't like with this. If you but 3 tickets, then we will send 3 different emails. I would instead send this email after creating the physical_tickets in one single email 🤔

Added mailer method to send ticket confirmation email along with the attached pdf for ticket.
@siddhantbajaj
Copy link
Contributor Author

screen shot 2017-07-26 at 11 32 42 am

Copy link
Member

@Ana06 Ana06 left a comment

Choose a reason for hiding this comment

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

LGTM 😻

@Ana06 Ana06 merged commit edd0024 into openSUSE:master Jul 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants