Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

Use Velocity to generate email #534

Closed
wants to merge 7 commits into from
Closed

Use Velocity to generate email #534

wants to merge 7 commits into from

Conversation

seanf
Copy link
Contributor

@seanf seanf commented Jul 11, 2014

No description provided.

carlosmunoz
carlosmunoz reviewed Jul 13, 2014
View reviewed changes
zanata-war/src/main/java/org/zanata/action/NewProfileAction.java
try {
EmailBuilder builder = new EmailBuilder(emailContext);
InternetAddress to = new InternetAddress(this.email, this.name, UTF_8.name());
builder.sendMessage(new EmailActivationEmailStrategy(key), to, null);
Copy link
Member

Choose a reason for hiding this comment

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

Should this be going through the EmailService instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, it could, but right now I'm just trying for a 1 to 1 conversion, so initially only the parts which already used EmailService are using it.

Besides, the EmailStrategies and EmailBuilder contain all the smarts, EmailService doesn't really bring anything useful to the party.

carlosmunoz
carlosmunoz reviewed Jul 14, 2014
View reviewed changes
zanata-war/src/main/java/org/zanata/i18n/Messages.java
// JSF can't handle varargs, hence the need for these overloaded methods:
public String format(String key, Object arg1) {
return format(key, new Object[] {arg1});
}
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit of a hole in EL. I wonder if/when they plan to address this.

@carlosmunoz
Copy link
Member

Review finished.

@seanf
Refactor email-handling code and fix injection of mailSession
bae62c5 
Eliminate redundant template "email_activation.vm".
Rename EmailUtil -> Addresses and use it more.
Rename EmailBuilderStrategy -> EmailStrategy.
Move all EmailStrategy classes to top-level under org.zanata.email.
Ensure that all emails go through EmailService and get FacesMessages
   from Msgs so that they can be localised.
@carlosmunoz
Copy link
Member

commit bae62c5 looks good 👍

@djansen-redhat
Copy link
Contributor

  • Clicking Help in the toolbar gets: 15:04:49,076 SEVERE javax.enterprise.resource.webcontainer.jsf.application Error Rendering View[/help/view.xhtml]: javax.el.ELException: org.jboss.seam.RequiredException: @ In attribute requires non-null value: sendEmail.languageJoinUpdateRoleAction
  • Viewing the email from a reset password any action shows only "Please use an HTML-capable email client to read this message."

@seanf
Copy link
Contributor Author

seanf commented Jul 18, 2014

It looks like the Reply-To header is also the wrong way around.

@seanf
Copy link
Contributor Author

seanf commented Jul 18, 2014

@djansen-redhat The above 3 bugs should be handled by these commits.

@djansen-redhat
Copy link
Contributor

✅ I've tested this, only a review of the last few commits is required and it can be merged.

@seanf
Copy link
Contributor Author

seanf commented Jul 23, 2014

@carlosmunoz could you please take a look at the last four commits?

@carlosmunoz
Copy link
Member

👍 Reviewed last 4 commits.

seanf added a commit that referenced this pull request Jul 24, 2014
@seanf
Use Velocity to generate email
dee1428 
#534

Squashed commit of the following:

commit cd0fbee
Author: Sean Flanigan <sflaniga@redhat.com>
Date:   Tue Jul 22 14:14:35 2014 +1000

    Update unit test: HTML should appear after text

commit 58f956a
Author: Sean Flanigan <sflaniga@redhat.com>
Date:   Fri Jul 18 18:10:50 2014 +1000

    Add @autocreate to injected components

commit d035448
Author: Sean Flanigan <sflaniga@redhat.com>
Date:   Fri Jul 18 18:08:27 2014 +1000

    Construct Reply-To correctly

commit 8ee323c
Author: Sean Flanigan <sflaniga@redhat.com>
Date:   Fri Jul 18 17:58:24 2014 +1000

    Add HTML mail part after text; produce text version of the HTML body

commit bae62c5
Author: Sean Flanigan <sflaniga@redhat.com>
Date:   Mon Jul 14 17:13:36 2014 +1000

    Refactor email-handling code and fix injection of mailSession

    Eliminate redundant template "email_activation.vm".
    Rename EmailUtil -> Addresses and use it more.
    Rename EmailBuilderStrategy -> EmailStrategy.
    Move all EmailStrategy classes to top-level under org.zanata.email.
    Ensure that all emails go through EmailService and get FacesMessages
       from Msgs so that they can be localised.

commit 8365f53
Author: Sean Flanigan <sflaniga@redhat.com>
Date:   Mon Jul 14 12:22:28 2014 +1000

    Make EmailBuilder into a component

commit 6f9ec47
Author: Sean Flanigan <sflaniga@redhat.com>
Date:   Fri Jul 11 12:46:36 2014 +1000

    Use Velocity to generate email
@seanf
Copy link
Contributor Author

seanf commented Jul 24, 2014

Merged with

git merge --squash origin/velocity-email; git commit

@seanf seanf closed this Jul 24, 2014
@seanf seanf deleted the velocity-email branch July 24, 2014 00:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@carlosmunoz carlosmunoz

Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@seanf @carlosmunoz @djansen-redhat @davidmason