So close.
With an invalid id and a valid pmc, there is some missing logic constructing
the icla-invalid-id message:
...
With this message, been notified that your ICLA has been filed.
...
Please review this patch and I'll push it.
diff --git a/www/secretary/workbench/views/actions/icla.json.rb
b/www/secretary/workbench/views/actions/icla.json.rb
index a65f0d0..ccfd9ac 100644
--- a/www/secretary/workbench/views/actions/icla.json.rb
+++ b/www/secretary/workbench/views/actions/icla.json.rb
@@ -114,7 +114,15 @@ end
# send confirmation email
task "email #@email" do
- # chose reply based on whether or not the project/userid info was provided
+ # set up notify for body of message
+ if @pmc
+ @notify = "the #{@pmc.display_name} PMC has"
+ if @podling
+ @notify.sub! /has$/, "and the #{@podling.display_name} podling have"
+ end
+ end
+
+ # choose reply message based on whether or not the project/userid info was
provided
if @user and not @user.empty?
if @valid_user
reply = 'icla-account-requested.erb'
@@ -122,12 +130,6 @@ task "email #@email" do
reply = 'icla-invalid-id.erb'
end
elsif @pmc
- @notify = "the #{@pmc.display_name} PMC has"
-
- if @podling
- @notify.sub! /has$/, "and the #{@podling.display_name} podling have"
- end
-
reply = 'icla-pmc-notified.erb'
else
reply = 'icla.erb'
> On Jul 18, 2017, at 7:35 AM, Sam Ruby <[email protected]> wrote:
>
> On Tue, Jul 18, 2017 at 9:44 AM, Craig Russell <[email protected]> wrote:
>> Huge improvement.
>>
>> Just one small issue and one question:
>>
>> 1. When the user id is invalid, and there is no project, the File button
>> should not be active.
>
> Fixed:
>
> https://github.com/apache/whimsy/commit/cb0bf176c4f7c2410f45cd8c04b88263456d2364
>
>> 2. When there is a project identified, the message does not have the
>> Reply-to field set. Does this happen automatically or should it be done
>> explicitly?
>
> That was a bug. Also fixed:
>
> https://github.com/apache/whimsy/commit/ec5de37b0a75f6ddadf4967a676260faeac9c14b
>
>> It's much more usable now.
>>
>> Thanks,
>>
>> Craig
>
> - Sam Ruby
>
>>> On Jul 18, 2017, at 5:18 AM, Sam Ruby <[email protected]> wrote:
>>>
>>> On Mon, Jul 17, 2017 at 11:04 PM, Craig Russell <[email protected]>
>>> wrote:
>>>>
>>>>> On Jul 17, 2017, at 6:37 PM, Sam Ruby <[email protected]> wrote:
>>>>>
>>>>> #1 is unrelated, but also now fixed.
>>>>>
>>>>> All three (as well as always validating the project) should be fixed by:
>>>>>
>>>>> https://github.com/apache/whimsy/commit/a1bbdf685b032dba1e074ded42aab1d9b2649b10
>>>>
>>>> Still have invalid apache id causing:
>>>> Match the required format
>>>> when File is pressed.
>>>
>>> Fixed:
>>> https://github.com/apache/whimsy/commit/358b63a77893bc2ab3f4773a10690011ac4032e7
>>>
>>>>> - Sam Ruby
>>>>>
>>>>> P.S. Should project be a dropdown?
>>>>
>>>> As long as it is subject to type-autocomplete, YES!
>>>>
>>>> As it is now, on Safari Version 10.1.1 (11603.2.5), it proposes an
>>>> autocomplete but when tabbing out of the field it doesn't autocomplete. So
>>>> I can verify that the project I type is valid, it doesn't help when I
>>>> start to type the name of the podling... All I can do is to finish typing
>>>> without it refusing to autocomplete.
>>>
>>> Let me know if this is to your liking:
>>>
>>> https://github.com/apache/whimsy/commit/8fdbc1fdcb57f69c1128169c400f9eab0c2b7821
>>>
>>>> Craig
>>>
>>> - Sam Ruby
>>>
>>>>> On Mon, Jul 17, 2017 at 9:13 PM, Craig Russell <[email protected]>
>>>>> wrote:
>>>>>> Oops, that broke it.
>>>>>>
>>>>>> 1. Podlings are not recognized.
>>>>>>
>>>>>> With just project filled with juneau (or any podling I tried)
>>>>>> Warning: juneau is not an active PMC or podling
>>>>>>
>>>>>> With just project filled with incubator:
>>>>>> Warning: incubator is not an active PMC or podling
>>>>>>
>>>>>> 2. If an invalid id is entered, the File button is enabled but it fails
>>>>>> with a tooltip: Match the requested format
>>>>>>
>>>>>> 3. If a valid id is entered, the Vote Link field is always required,
>>>>>> even it the id is subsequently erased. If the File button is pressed,
>>>>>> the tooltip asks Fill out this field. Reloading the page resets it.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Craig
>>>>>>
>>>>>>
>>>>>>> On Jul 17, 2017, at 5:58 PM, Craig Russell <[email protected]> wrote:
>>>>>>>
>>>>>>> Hi Sam,
>>>>>>>
>>>>>>>> On Jul 17, 2017, at 1:27 PM, Sam Ruby <[email protected]> wrote:
>>>>>>>>
>>>>>>>> On Mon, Jul 17, 2017 at 3:18 PM, Craig Russell <[email protected]>
>>>>>>>> wrote:
>>>>>>>>> I've checked in the proposed icla-invalid-id message. I cannot figure
>>>>>>>>> out how to activate it.
>>>>>>>>>
>>>>>>>>> There still needs to be a check in icla.js.rb for invalid id. But if
>>>>>>>>> it is illegal, the (File) button should be enabled. This allows
>>>>>>>>> icla.json.rb to construct the appropriate email.
>>>>>>>>>
>>>>>>>>> Perhaps
>>>>>>>>>
>>>>>>>>> if @user and not @checked
>>>>>>>>> reply = 'icla-invalid-id.erb'
>>>>>>>>> else if ...
>>>>>>>>>
>>>>>>>>> But then later, instead of
>>>>>>>>> if @user and not @user.empty?
>>>>>>>>>
>>>>>>>>> add a check for @checked
>>>>>>>>> if @user and @checked and not @user.empty?
>>>>>>>>>
>>>>>>>>> Seems easy enough but I'm not quite good enough with js.rb.json stuff.
>>>>>>>>
>>>>>>>> If I'm understanding correctly:
>>>>>>>>
>>>>>>>> On the client side, you no longer want the File button to be disabled
>>>>>>>> if the provided user id is invalid. Furthermore, if the user id is
>>>>>>>> invalid, the validity of the project and votelink fields should no
>>>>>>>> longer affect whether the File button is enabled or not.
>>>>>>>
>>>>>>> Very close! The project field should always be validated and the (File)
>>>>>>> button disabled if so. If the id is valid, the vote link field should
>>>>>>> still be validated. But close enough. If I get a red project alert I'll
>>>>>>> fix it regardless of whether (File) is enabled.
>>>>>>>>
>>>>>>>> On the server side, the user id provided needs to be checked, and if
>>>>>>>> it doesn't match the regular expression or isn't available, then a
>>>>>>>> different reply template should be used and the new account request
>>>>>>>> should be skipped.
>>>>>>>
>>>>>>> Yes!
>>>>>>>>
>>>>>>>> The following change should do the above:
>>>>>>>>
>>>>>>>> https://github.com/apache/whimsy/commit/6d6409756c67a95216ecf670ea7baded064bfcf4
>>>>>>>>
>>>>>>>> Note: I made a small tweak to the invalid-id-request reply to clarify
>>>>>>>> that the id must be lowercase.
>>>>>>>
>>>>>>> Cool. If an id request comes in that is otherwise ok, I simply
>>>>>>> lower-case it and submit. In other words, I won't refuse an id if it's
>>>>>>> just case.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Craig
>>>>>>>>
>>>>>>>>> Craig
>>>>>>>>
>>>>>>>> - Sam Ruby
>>>>>>>>
>>>>>>>>>> On Jul 17, 2017, at 11:01 AM, Craig Russell <[email protected]>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>> Currently the secmail tool properly checks a proposed id and
>>>>>>>>>> highlights it in red if it does not conform. Either it is already in
>>>>>>>>>> use or it is not alphabetic followed by at least two alphanumeric
>>>>>>>>>> characters. If the id is illegal, the tool refuses to file the icla.
>>>>>>>>>> Then everyone complains that the account was not created.
>>>>>>>>>>
>>>>>>>>>> It comes up often enough that I'd like a different way of handling
>>>>>>>>>> this case.
>>>>>>>>>>
>>>>>>>>>> If the name is illegal, I'd like to file the icla but send a
>>>>>>>>>> different message.
>>>>>>>>>>
>>>>>>>>>> I'll work on the message and then work on the processing to send it.
>>>>>>>>>>
>>>>>>>>>> Craig
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Craig L Russell
>>>>>>>>>> Secretary, Apache Software Foundation
>>>>>>>>>> [email protected] <mailto:[email protected]> http://db.apache.org/jdo
>>>>>>>>>> <http://db.apache.org/jdo>
>>>>>>>>>
>>>>>>>>> Craig L Russell
>>>>>>>>> Secretary, Apache Software Foundation
>>>>>>>>> [email protected] <mailto:[email protected]> http://db.apache.org/jdo
>>>>>>>>> <http://db.apache.org/jdo>
>>>>>>>
>>>>>>> Craig L Russell
>>>>>>> Architect
>>>>>>> [email protected]
>>>>>>> P.S. A good JDO? O, Gasp!
>>>>>>
>>>>>> Craig L Russell
>>>>>> Secretary, Apache Software Foundation
>>>>>> [email protected] http://db.apache.org/jdo
>>>>>>
>>>>
>>>> Craig L Russell
>>>> Architect
>>>> [email protected]
>>>> P.S. A good JDO? O, Gasp!
>>>>
>>>>
>>>>
>>>>
>>>>
>>
>> Craig L Russell
>> Architect
>> [email protected]
>> P.S. A good JDO? O, Gasp!
>>
>>
>>
>>
>>
Craig L Russell
Architect
[email protected]
P.S. A good JDO? O, Gasp!