You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@whimsical.apache.org by Craig Russell <cr...@oracle.com> on 2016/09/15 17:52:49 UTC

Secmail icla

I changed the email address of the icla and the original was not cc: on the email.


Craig L Russell
Architect
craig.russell@oracle.com
P.S. A good JDO? O, Gasp!






Re: Secmail icla

Posted by Sam Ruby <ru...@intertwingly.net>.
On Thu, Sep 15, 2016 at 8:54 PM, Sam Ruby <ru...@intertwingly.net> wrote:
>
> CCLA and SG can be done quickly by copying the first three tasks of
> ICLA and the forms from the workbench.

I've roughed in both.  They have only partially been tested.  Give
them a try and let me know if there are any problems.

- Sam Ruby

Re: Secmail icla

Posted by Sam Ruby <ru...@intertwingly.net>.
On Thu, Sep 15, 2016 at 8:24 PM, Craig Russell <cr...@oracle.com> wrote:
> Hi Sam,
>
>> On Sep 15, 2016, at 5:01 PM, Sam Ruby <ru...@intertwingly.net> wrote:
>>
>> On Thu, Sep 15, 2016 at 3:24 PM, Sam Ruby <ru...@intertwingly.net> wrote:
>>> As luck would have it, rewriting how emails are constructed was going
>>> to be next on my list.  If you look at icla.json.rb, the svn tasks are
>>> fairly clean and the mail tasks are a bit brute force, and contain a
>>> lot of common code.  This code would be required for each of the
>>> actions, so I plan to factor it out into a common method.
>>>
>>> I'll look at it later this afternoon.  Part of that commit will be
>>> support for the various reject actions (incomplete, unsigned, etc),
>>> which will become trivial (jump immediately to the tasklist with no
>>> form, the task consists of a single email which you can browse before
>>> proceeding).
>>
>> Done for now.  Here's the commit:
>>
>> https://github.com/apache/whimsy/commit/6aefcd7ce2121a389588dd7279cfab302ed48b2c
>
> W O W lots of code in there. I’m just a bit surprised there is not more common code in unsigned.json.rb, pubkey.json.rb, incomplete.json.rb. I thought there would be more in message.rb that could be reused more easily. I’m a big fan of DRY. ;-)

If those three files were symlinked together, the one remaining
difference (the name of the template) could possibly be extracted from
the URL, which in turn is available in the env variable.  An example
of what can be found in the env variable:

https://whimsy.apache.org/racktest

>> For what it is worth, the documentation on the FromField can be found here:
>>
>> http://www.rubydoc.info/github/mikel/mail/Mail
>>
>> I'll state that I find the API that gem provides to be a little...
>> quirky.  Apparently email can have multiple from fields (who knew?)
>
> Not I. Never saw any mail from more than one address.
>
>> and some of the methods provide the full email string (including name
>> and optional comment) and others provide only the email address
>> itself.  Add to that the fact that the secmail tool will allow you to
>> edit the cc and bcc fields, and merging addresses from the original
>> email and the input from the form and there being multiple fields
>> (from, to, cc, bcc) and you have a bit of a mess.
>
> Certainly not easy to just add a bit of logic. I tried that. :(

Hopefully the hard stuff continues to get factored out, leaving just
the "business logic" in places like:

https://github.com/apache/whimsy/blob/master/www/secmail/views/forms/icla.js.rb
https://github.com/apache/whimsy/blob/master/www/secmail/views/actions/icla.json.rb

You saw how easy it is to add a button that does something useful.
Most of the credit goes to React.js.  In your case,you just set three
variables, and React.js figured out what updates to make to the DOM to
reflect the new values.

I'd like to extend that.  Don't just fill in the real name, but also
fill in the file name.  And not just initially, but update as the real
name changes.  Of course, once you have entered the real name you can
go and change the file name should you be so inclined, but that rarely
would be necessary.

I'd like to explore tailoring the icla responses depending on the
input.  Right now it is fairly generic.  A different body could be
provided for cases where there is a project (PMC or podling) and yet
another one if there is a userid and votelink.  The beauty of this is
that you won't have to do anything to select which template is used.

>> In any case, this mess is encapsulated now, so once debugged it
>> hopefully will work consistently for all replies.
>
> Looks good so far.
>
> Do you have a schedule for the rest of the doc types so we can get rid of duplicate handling of the incoming mails?

I've got a wedding to go to this weekend, so probably early next week.
CCLA and SG can be done quickly by copying the first three tasks of
ICLA and the forms from the workbench.  I'd have to research to
remember the membership application process, but that should also be
fairly quick, and not something we will need anyway for a few months.

- Sam Ruby

> Thanks!
>
> Craig
>>
>> - Sam Ruby
>
> Craig L Russell
> Architect
> craig.russell@oracle.com
> P.S. A good JDO? O, Gasp!

Re: Secmail icla

Posted by sebb <se...@gmail.com>.
On 16 September 2016 at 01:24, Craig Russell <cr...@oracle.com> wrote:
> Hi Sam,
>
>> On Sep 15, 2016, at 5:01 PM, Sam Ruby <ru...@intertwingly.net> wrote:
>>
>> On Thu, Sep 15, 2016 at 3:24 PM, Sam Ruby <ru...@intertwingly.net> wrote:
>>> As luck would have it, rewriting how emails are constructed was going
>>> to be next on my list.  If you look at icla.json.rb, the svn tasks are
>>> fairly clean and the mail tasks are a bit brute force, and contain a
>>> lot of common code.  This code would be required for each of the
>>> actions, so I plan to factor it out into a common method.
>>>
>>> I'll look at it later this afternoon.  Part of that commit will be
>>> support for the various reject actions (incomplete, unsigned, etc),
>>> which will become trivial (jump immediately to the tasklist with no
>>> form, the task consists of a single email which you can browse before
>>> proceeding).
>>
>> Done for now.  Here's the commit:
>>
>> https://github.com/apache/whimsy/commit/6aefcd7ce2121a389588dd7279cfab302ed48b2c
>
> W O W lots of code in there. I’m just a bit surprised there is not more common code in unsigned.json.rb, pubkey.json.rb, incomplete.json.rb. I thought there would be more in message.rb that could be reused more easily. I’m a big fan of DRY. ;-)
>
>>
>> For what it is worth, the documentation on the FromField can be found here:
>>
>> http://www.rubydoc.info/github/mikel/mail/Mail
>>
>> I'll state that I find the API that gem provides to be a little...
>> quirky.  Apparently email can have multiple from fields (who knew?)
>
> Not I. Never saw any mail from more than one address.

[FTR]

https://tools.ietf.org/html/rfc4021#section-2.1.2
says:

Specifies the author(s) of the message; that is, the mailbox(es)
of the person(s) or system(s) responsible for the writing of the
message.

The Sender is defined as:

Specifies the mailbox of the agent responsible for the actual
transmission of the message.


>> and some of the methods provide the full email string (including name
>> and optional comment) and others provide only the email address
>> itself.  Add to that the fact that the secmail tool will allow you to
>> edit the cc and bcc fields, and merging addresses from the original
>> email and the input from the form and there being multiple fields
>> (from, to, cc, bcc) and you have a bit of a mess.
>
> Certainly not easy to just add a bit of logic. I tried that. :(
>>
>> In any case, this mess is encapsulated now, so once debugged it
>> hopefully will work consistently for all replies.
>
> Looks good so far.
>
> Do you have a schedule for the rest of the doc types so we can get rid of duplicate handling of the incoming mails?
>
> Thanks!
>
> Craig
>>
>> - Sam Ruby
>
> Craig L Russell
> Architect
> craig.russell@oracle.com
> P.S. A good JDO? O, Gasp!
>
>
>
>
>

Re: Secmail icla

Posted by Craig Russell <cr...@oracle.com>.
Hi Sam,

> On Sep 15, 2016, at 5:01 PM, Sam Ruby <ru...@intertwingly.net> wrote:
> 
> On Thu, Sep 15, 2016 at 3:24 PM, Sam Ruby <ru...@intertwingly.net> wrote:
>> As luck would have it, rewriting how emails are constructed was going
>> to be next on my list.  If you look at icla.json.rb, the svn tasks are
>> fairly clean and the mail tasks are a bit brute force, and contain a
>> lot of common code.  This code would be required for each of the
>> actions, so I plan to factor it out into a common method.
>> 
>> I'll look at it later this afternoon.  Part of that commit will be
>> support for the various reject actions (incomplete, unsigned, etc),
>> which will become trivial (jump immediately to the tasklist with no
>> form, the task consists of a single email which you can browse before
>> proceeding).
> 
> Done for now.  Here's the commit:
> 
> https://github.com/apache/whimsy/commit/6aefcd7ce2121a389588dd7279cfab302ed48b2c

W O W lots of code in there. I’m just a bit surprised there is not more common code in unsigned.json.rb, pubkey.json.rb, incomplete.json.rb. I thought there would be more in message.rb that could be reused more easily. I’m a big fan of DRY. ;-)

> 
> For what it is worth, the documentation on the FromField can be found here:
> 
> http://www.rubydoc.info/github/mikel/mail/Mail
> 
> I'll state that I find the API that gem provides to be a little...
> quirky.  Apparently email can have multiple from fields (who knew?)

Not I. Never saw any mail from more than one address.

> and some of the methods provide the full email string (including name
> and optional comment) and others provide only the email address
> itself.  Add to that the fact that the secmail tool will allow you to
> edit the cc and bcc fields, and merging addresses from the original
> email and the input from the form and there being multiple fields
> (from, to, cc, bcc) and you have a bit of a mess.

Certainly not easy to just add a bit of logic. I tried that. :(
> 
> In any case, this mess is encapsulated now, so once debugged it
> hopefully will work consistently for all replies.

Looks good so far. 

Do you have a schedule for the rest of the doc types so we can get rid of duplicate handling of the incoming mails?

Thanks!

Craig
> 
> - Sam Ruby

Craig L Russell
Architect
craig.russell@oracle.com
P.S. A good JDO? O, Gasp!






Re: Secmail icla

Posted by Sam Ruby <ru...@intertwingly.net>.
On Thu, Sep 15, 2016 at 3:24 PM, Sam Ruby <ru...@intertwingly.net> wrote:
> As luck would have it, rewriting how emails are constructed was going
> to be next on my list.  If you look at icla.json.rb, the svn tasks are
> fairly clean and the mail tasks are a bit brute force, and contain a
> lot of common code.  This code would be required for each of the
> actions, so I plan to factor it out into a common method.
>
> I'll look at it later this afternoon.  Part of that commit will be
> support for the various reject actions (incomplete, unsigned, etc),
> which will become trivial (jump immediately to the tasklist with no
> form, the task consists of a single email which you can browse before
> proceeding).

Done for now.  Here's the commit:

https://github.com/apache/whimsy/commit/6aefcd7ce2121a389588dd7279cfab302ed48b2c

For what it is worth, the documentation on the FromField can be found here:

http://www.rubydoc.info/github/mikel/mail/Mail

I'll state that I find the API that gem provides to be a little...
quirky.  Apparently email can have multiple from fields (who knew?)
and some of the methods provide the full email string (including name
and optional comment) and others provide only the email address
itself.  Add to that the fact that the secmail tool will allow you to
edit the cc and bcc fields, and merging addresses from the original
email and the input from the form and there being multiple fields
(from, to, cc, bcc) and you have a bit of a mess.

In any case, this mess is encapsulated now, so once debugged it
hopefully will work consistently for all replies.

- Sam Ruby

Re: Secmail icla

Posted by Sam Ruby <ru...@intertwingly.net>.
As luck would have it, rewriting how emails are constructed was going
to be next on my list.  If you look at icla.json.rb, the svn tasks are
fairly clean and the mail tasks are a bit brute force, and contain a
lot of common code.  This code would be required for each of the
actions, so I plan to factor it out into a common method.

I'll look at it later this afternoon.  Part of that commit will be
support for the various reject actions (incomplete, unsigned, etc),
which will become trivial (jump immediately to the tasklist with no
form, the task consists of a single email which you can browse before
proceeding).

- Sam Ruby

On Thu, Sep 15, 2016 at 2:56 PM, Craig Russell <cr...@oracle.com> wrote:
> Here’s a patch that doesn’t work:
>
> I tried to find any documentation on the Message class but failed to find which property has the original “from” address.
>
> So do I need to convert the message:from into an array? I tried
>
> +  cc += [message.from] <== line 120
>
> but no luck
>
> Any ideas?
>
> Craig
>
>
> --- a/www/secmail/views/actions/icla.json.rb
> +++ b/www/secmail/views/actions/icla.json.rb
> @@ -117,6 +117,7 @@ task "email #@email" do
>    # adjust copy lists
>    cc = mail.cc # from the template
>    cc += message.cc if message.cc # from the email message
> +  cc += message.from <== line 120
>    cc << "private@#{pmc.mail_list}.apache.org" if pmc # copy pmc
>    cc << podling.private_mail_list if podling # copy podling
>    mail.cc = cc.uniq
>
> #<TypeError: no implicit conversion of Mail::Field into Array>
> /Users/clr/apache/git/whimsy/www/secmail/views/actions/icla.json.rb:120:in `block in _evaluate'
> /Users/clr/apache/git/whimsy/www/secmail/tasks.rb:9:in `task'
> /Users/clr/apache/git/whimsy/www/secmail/views/actions/icla.json.rb:113:in `_evaluate'
> /Users/clr/apache/git/whimsy/www/secmail/server.rb:58:in `block in <top (required)>'
> /usr/local/lib/ruby/gems/2.3.0/gems/passenger-5.0.30/src/ruby_supportlib/phusion_passenger/rack/out_of_band_gc.rb:48:in `call'
> /Users/clr/apache/git/whimsy/lib/whimsy/asf/rack.rb:143:in `call'
> /Users/clr/apache/git/whimsy/lib/whimsy/asf/rack.rb:83:in `call'
> /Users/clr/apache/git/whimsy/lib/whimsy/asf/rack.rb:217:in `call'
> /usr/local/lib/ruby/gems/2.3.0/gems/passenger-5.0.30/src/ruby_supportlib/phusion_passenger/rack/thread_handler_extension.rb:97:in `process_request'
> /usr/local/lib/ruby/gems/2.3.0/gems/passenger-5.0.30/src/ruby_supportlib/phusion_passenger/request_handler/thread_handler.rb:160:in `accept_and_process_next_request'
> /usr/local/lib/ruby/gems/2.3.0/gems/passenger-5.0.30/src/ruby_supportlib/phusion_passenger/request_handler/thread_handler.rb:113:in `main_loop'
> /usr/local/lib/ruby/gems/2.3.0/gems/passenger-5.0.30/src/ruby_supportlib/phusion_passenger/request_handler.rb:416:in `block (3 levels) in start_threads'
> /usr/local/lib/ruby/gems/2.3.0/gems/passenger-5.0.30/src/ruby_supportlib/phusion_passenger/utils.rb:113:in `block in create_thread_and_abort_on_exception'
>
>> On Sep 15, 2016, at 10:52 AM, Craig Russell <cr...@oracle.com> wrote:
>>
>> I changed the email address of the icla and the original was not cc: on the email.
>>
>>
>>
>>
>
> Craig L Russell
> clr@apache.org
>
>

Re: Secmail icla

Posted by Craig Russell <cr...@oracle.com>.
Here’s a patch that doesn’t work:

I tried to find any documentation on the Message class but failed to find which property has the original “from” address.

So do I need to convert the message:from into an array? I tried 

+  cc += [message.from] <== line 120

but no luck

Any ideas?

Craig


--- a/www/secmail/views/actions/icla.json.rb
+++ b/www/secmail/views/actions/icla.json.rb
@@ -117,6 +117,7 @@ task "email #@email" do
   # adjust copy lists
   cc = mail.cc # from the template
   cc += message.cc if message.cc # from the email message
+  cc += message.from <== line 120
   cc << "private@#{pmc.mail_list}.apache.org" if pmc # copy pmc
   cc << podling.private_mail_list if podling # copy podling
   mail.cc = cc.uniq

#<TypeError: no implicit conversion of Mail::Field into Array>
/Users/clr/apache/git/whimsy/www/secmail/views/actions/icla.json.rb:120:in `block in _evaluate'
/Users/clr/apache/git/whimsy/www/secmail/tasks.rb:9:in `task'
/Users/clr/apache/git/whimsy/www/secmail/views/actions/icla.json.rb:113:in `_evaluate'
/Users/clr/apache/git/whimsy/www/secmail/server.rb:58:in `block in <top (required)>'
/usr/local/lib/ruby/gems/2.3.0/gems/passenger-5.0.30/src/ruby_supportlib/phusion_passenger/rack/out_of_band_gc.rb:48:in `call'
/Users/clr/apache/git/whimsy/lib/whimsy/asf/rack.rb:143:in `call'
/Users/clr/apache/git/whimsy/lib/whimsy/asf/rack.rb:83:in `call'
/Users/clr/apache/git/whimsy/lib/whimsy/asf/rack.rb:217:in `call'
/usr/local/lib/ruby/gems/2.3.0/gems/passenger-5.0.30/src/ruby_supportlib/phusion_passenger/rack/thread_handler_extension.rb:97:in `process_request'
/usr/local/lib/ruby/gems/2.3.0/gems/passenger-5.0.30/src/ruby_supportlib/phusion_passenger/request_handler/thread_handler.rb:160:in `accept_and_process_next_request'
/usr/local/lib/ruby/gems/2.3.0/gems/passenger-5.0.30/src/ruby_supportlib/phusion_passenger/request_handler/thread_handler.rb:113:in `main_loop'
/usr/local/lib/ruby/gems/2.3.0/gems/passenger-5.0.30/src/ruby_supportlib/phusion_passenger/request_handler.rb:416:in `block (3 levels) in start_threads'
/usr/local/lib/ruby/gems/2.3.0/gems/passenger-5.0.30/src/ruby_supportlib/phusion_passenger/utils.rb:113:in `block in create_thread_and_abort_on_exception'

> On Sep 15, 2016, at 10:52 AM, Craig Russell <cr...@oracle.com> wrote:
> 
> I changed the email address of the icla and the original was not cc: on the email.
> 
> 
> 
> 

Craig L Russell
clr@apache.org