parser: Don't update person.name when they comment

Message ID 20190108123834.2225-6-alialnu@mellanox.com
State Accepted
Headers show
Series
  • parser: Don't update person.name when they comment
Related show

Commit Message

Ali Alnubani Jan. 8, 2019, 12:38 p.m.
This patch modifies parser to not update the Person object
when commenting, but only when a new submission is received.

The reason behind this change is to always respect
the name set in git configurations.

Partially revert d018cd5be5e007
("parsemail: Always update Person.email")

Suggested-by: Thomas Monjalon <thomas@monjalon.net>
Signed-off-by: Ali Alnubani <alialnu@mellanox.com>
---
 patchwork/parser.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Stephen Finucane Feb. 25, 2019, 11:15 a.m. | #1
On Tue, 2019-01-08 at 12:38 +0000, Ali Alnubani wrote:
> This patch modifies parser to not update the Person object
> when commenting, but only when a new submission is received.
> 
> The reason behind this change is to always respect
> the name set in git configurations.
> 
> Partially revert d018cd5be5e007
> ("parsemail: Always update Person.email")
> 
> Suggested-by: Thomas Monjalon <thomas@monjalon.net>
> Signed-off-by: Ali Alnubani <alialnu@mellanox.com>
> ---
>  patchwork/parser.py | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/patchwork/parser.py b/patchwork/parser.py
> index 946b668..c7297ae 100644
> --- a/patchwork/parser.py
> +++ b/patchwork/parser.py
> @@ -313,7 +313,7 @@ def find_series(project, mail, author):
>      return _find_series_by_markers(project, mail, author)
>  
>  
> -def get_or_create_author(mail):
> +def get_or_create_author(mail, save_required=True):
>      from_header = clean_header(mail.get('From'))
>  
>      if not from_header:
> @@ -361,10 +361,10 @@ def get_or_create_author(mail):
>      person = Person.objects.get_or_create(email__iexact=email,
>                                            defaults={'name': name,
>                                                      'email': email})[0]
> -
> -    if name and name != person.name:  # use the latest provided name
> -        person.name = name
> -        person.save()
> +    if save_required:
> +        if name and name != person.name:  # use the latest provided name
> +            person.name = name
> +            person.save()
>  
>      return person
>  
> @@ -1152,7 +1152,7 @@ def parse_mail(mail, list_id=None):
>      if not submission:
>          return
>  
> -    author = get_or_create_author(mail)
> +    author = get_or_create_author(mail, save_required=is_comment)

This seems backwards: surely we should _not_ be saving if it's a
comment? Assuming I'm correct in my reading of this, perhaps some unit
tests wouldn't go astray.

Stephen

>  
>      try:
>          comment = Comment.objects.create(

Patch

diff --git a/patchwork/parser.py b/patchwork/parser.py
index 946b668..c7297ae 100644
--- a/patchwork/parser.py
+++ b/patchwork/parser.py
@@ -313,7 +313,7 @@  def find_series(project, mail, author):
     return _find_series_by_markers(project, mail, author)
 
 
-def get_or_create_author(mail):
+def get_or_create_author(mail, save_required=True):
     from_header = clean_header(mail.get('From'))
 
     if not from_header:
@@ -361,10 +361,10 @@  def get_or_create_author(mail):
     person = Person.objects.get_or_create(email__iexact=email,
                                           defaults={'name': name,
                                                     'email': email})[0]
-
-    if name and name != person.name:  # use the latest provided name
-        person.name = name
-        person.save()
+    if save_required:
+        if name and name != person.name:  # use the latest provided name
+            person.name = name
+            person.save()
 
     return person
 
@@ -1152,7 +1152,7 @@  def parse_mail(mail, list_id=None):
     if not submission:
         return
 
-    author = get_or_create_author(mail)
+    author = get_or_create_author(mail, save_required=is_comment)
 
     try:
         comment = Comment.objects.create(