518510 - Can't enter username formula for incoming server in add new form

Status: RESOLVED FIXED

(Webtools :: ISPDB Server, defect, P1)

Description

[Patrick Fey]

In the "Add new" form there is only one username formula field. This seems to be the username formula associated with the outgoing server (Note: This is my guess after looking at the configuration preview). There is no way to enter a username formula associated with the incoming server.

Comment 1

[Blake Winton (:bwinton) (:☕️)]

Check out the ConfigForm in config/models.py.
For some reason we're excluding the incoming username formula.

I suspect it's because we should be using the same username formula for both incoming and outgoing, but if that's the case, we need to copy it over when the user submits the form.

Comment 2

[Blake Winton (:bwinton) (:☕️)]

(D'oh, forgot to change the "resolved" status before commenting.)

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

As per discussion, we've decide that we won't actually block on this.

Comment 4

[Lindauson]

I see that this Bug is not assigned to anyone. I would like to work on it...

Comment 6

[Ben Bucksch (:BenB)]

Lindauson, go ahead, we're accepting patches :)

Comment 7

[Ben Bucksch (:BenB)]

Note, the the XML element is empty, too:
<http://ispdb.mozillamessaging.com/export_xml/64>
The client actually treats them separate in the code, so this breaks TB.

Expected result:
Either
- a username formula textfield for the incoming server,
  just like we have for the outgoing server
- One text field for both, and it's populated to both elements in the XML.
  (This is based on the fact that most ISPs use the same username
  for both incoming and SMTP, to not confuse users, but disadvantage is
  that we wouldn't be able to support those fringe ISPs who differ there.
  I have seen ISPs who actually accept a certain form only one one server,
  but there typically has been an straightforward form that works on both.)

Comment 8

[Blake Winton (:bwinton) (:☕️)]

Attachment: A patch to add the incoming username, and fix the tests.

Let's try this patch.

Comment 10

[Ludovic Hirlimann [:Usul]]

David, ping for the review.

Comment 11

[David Ascher (:davida)]

Drive by:

+               incoming_authentication = incoming_authentication,

+               outgoing_authentication = outgoing_authentication,

that is suspicious.  I haven't looked at the context to know if we really want to make a shadow of a global, but i doubt it. =)



+        c2 = Domain.objects.filter(name=curr_domain)
+        if len(c2) == 1:
+            c2 = c2[0].config
+        else:
+            c2 = c

that looks like it could use a comment


I need a bit more time to actually do a real review though.

Comment 12

[Blake Winton (:bwinton) (:☕️)]

(In reply to comment #11)
> Drive by:
> +               incoming_authentication = incoming_authentication,
> +               outgoing_authentication = outgoing_authentication,
> that is suspicious.  I haven't looked at the context to know if we really want
> to make a shadow of a global, but i doubt it. =)

The context here is that we're passing the local variable into a constructor as a keyword parameter.

> +        c2 = Domain.objects.filter(name=curr_domain)
> +        if len(c2) == 1:
> +            c2 = c2[0].config
> +        else:
> +            c2 = c
> that looks like it could use a comment

Uh, yeah.  As soon as I figure out what I was hoping to do there, I'll definitely comment it.  :)

> I need a bit more time to actually do a real review though.

Okay.  Thanks for the drive-by.

Later,
Blake.

Comment 13

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 419348 [details] [diff] [review]
A patch to add the incoming username, and fix the tests.

Changing the request to Sancus, since he's doing some work in this area, and probably knows the code better than David or I at this point.  ;)

Comment 14

[Sergio Roberto]

This has been fixed in https://github.com/mozilla/ispdb/pull/5.

Comment 15

[Mark Banner (:standard8)]

Comment on attachment 419348 [details] [diff] [review]
A patch to add the incoming username, and fix the tests.

Removing now obsolete review request.