Patchwork models: Don't munge the 'From' field of patches

login
register
mail settings
Submitter Doug Anderson
Date Feb. 15, 2013, 11:57 p.m.
Message ID <1360972641-4086-1-git-send-email-dianders@chromium.org>
Download mbox | patch
Permalink /patch/220899/
State New
Headers show

Comments

Doug Anderson - Feb. 15, 2013, 11:57 p.m.
At the moment patchwork always uses the official submitter name (as
patchwork understands it) as the "From" for patches that you receive.
This isn't quite what users expect and has some unfortunate
consequences.

The biggest problem is that patchwork saves the "official" name for an
email address the first time it sees an email from them.  If that name
is wrong (or was missing) patchwork will be confused even if future
emails from this person are fixed.  There are similar problems if a
user changes his/her name (get married?).

It seems better to just have each patch report the actual "From" that
was used to send that patch.  We'll still return the submitter in
'X-Patchwork-Submitter' just in case someone wants it.

Reported-by: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Doug Anderson <dianders@chromium.org>
---
 apps/patchwork/models.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Jeremy Kerr - Feb. 16, 2013, 5:05 a.m.
Hi Doug,

> At the moment patchwork always uses the official submitter name (as
> patchwork understands it) as the "From" for patches that you receive.
> This isn't quite what users expect and has some unfortunate
> consequences.

Agreed.

> The biggest problem is that patchwork saves the "official" name for an
> email address the first time it sees an email from them.  If that name
> is wrong (or was missing) patchwork will be confused even if future
> emails from this person are fixed.  There are similar problems if a
> user changes his/her name (get married?).
>
> It seems better to just have each patch report the actual "From" that
> was used to send that patch.  We'll still return the submitter in
> 'X-Patchwork-Submitter' just in case someone wants it.

This change will cause the mbox download to show a different name than 
the web UI, which may also be confusing. How about we store the name in 
the Patch model (if it differs from the Person object) from the From header?

Cheers,


Jeremy
Doug Anderson - Feb. 18, 2013, 11:03 p.m.
Jeremy,

On Fri, Feb 15, 2013 at 9:05 PM, Jeremy Kerr <jk@ozlabs.org> wrote:
>> The biggest problem is that patchwork saves the "official" name for an
>> email address the first time it sees an email from them.  If that name
>> is wrong (or was missing) patchwork will be confused even if future
>> emails from this person are fixed.  There are similar problems if a
>> user changes his/her name (get married?).
>>
>> It seems better to just have each patch report the actual "From" that
>> was used to send that patch.  We'll still return the submitter in
>> 'X-Patchwork-Submitter' just in case someone wants it.
>
> This change will cause the mbox download to show a different name than the
> web UI, which may also be confusing. How about we store the name in the
> Patch model (if it differs from the Person object) from the From header?

In which part of the web UI?  I played with this and the "Headers"
still shows the proper (un-munged) From address.  Certainly the
"Submitter" at the top still shows the official patchwork idea of what
the person's name is but I'm not convinced that's a bad thing.
Overall patchwork has the concept of what the official name of a
submitter in several places (replies, the "Filters" UI, etc).  Having
the UI show a different name than the "From" mail header is not so
different than seeing a "reply" have a different name than the
"Acked-by" like that the reply contains...

Anyway, I'm happy to give your proposal a shot if you want.  Let me know...

-Doug

Patch

diff --git a/apps/patchwork/models.py b/apps/patchwork/models.py
index 86a5266..6b9baf7 100644
--- a/apps/patchwork/models.py
+++ b/apps/patchwork/models.py
@@ -281,13 +281,13 @@  class Patch(models.Model):
         mail['Subject'] = self.name
         mail['Date'] = email.utils.formatdate(
                         time.mktime(self.date.utctimetuple()))
-        mail['From'] = unicode(self.submitter)
+        mail['X-Patchwork-Submitter'] = unicode(self.submitter)
         mail['X-Patchwork-Id'] = str(self.id)
         mail['Message-Id'] = self.msgid
         mail.set_unixfrom('From patchwork ' + self.date.ctime())
 
 
-        copied_headers = ['To', 'Cc']
+        copied_headers = ['To', 'Cc', 'From']
         orig_headers = HeaderParser().parsestr(str(self.headers))
         for header in copied_headers:
             if header in orig_headers: