Summary

Fixed the signature in the email sent when the review request is closed by admin

Review Request #12029 — Created Feb. 3, 2022 and updated April 5, 2022, 1:26 a.m.

Information

Owner

sheenaNg

Repository

Review Board

Branch

release-4.0.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Currently, when users with admin rights close the review request,
the email template shows the wrong name in the signature at the
bottom of the e-mail content.

This change involves replacing the review_request.submitter
with user to display the name of the admin / user that
closed the review request instead of the owner of the
review request.

Testing Done

Added unit test to test_sending_email.py

I have also included the screenshots that show the changes:

This screenshot shows the TXT e-mail that is created when
I re-opened the review request and published it. My name is
in the signature of the e-email.
This screenshot shows the TXT e-mail that is created when
a user with admin rights closed the review request (discarded).
The name of the user with admin rights (superUser Admin) is
in the signature of the e-mail.

Commits

Summary	ID	Author
Fixed the signature in the email sent when the review request is closed by admin	f9a5d2799004930b7c5220acd404165e69f34170	sng06
Changed the review_request.submitter variable to user	737990f499cd7d73fab539744ea94a8479dedce2	sng06
Reverted changes in html file and changed the signature variable to user	73d71b7666cebc7c00c42c4a69375b12ccad5457	sng06
Reverted changes in html template as we want to preserve the author in the By field	e7e5225b43aa7d152b71b585fc8dc934320ff620	sng06
Added unit test to test changes of close email after fixing the signature	515b361b5fe1ddbae29ce5a991e0a889d199707f	sng06
Made changes to codes according to code review feedback received	9c3ba0baa111c96e7361337fdfee191a2145475f	sng06

Files

Issues

Description	From	Last Updated
I know some of the other things in here say "Tests", but all test docstrings should start with "Testing"	david	April 5, 2022, 1:25 a.m.
May be a minor thing, but you can shorten this, by including publish=True inside the self.create_review_request function.	gdehal	April 5, 2022, 1:25 a.m.
These can be self.assertIn('...', message.as_string()), which I think will give us better debug output if they fail.	david	April 5, 2022, 1:25 a.m.

flake8 passed.

JSHint passed.

Change Summary:

Added issues to get further clarification

Description:

		Currently, when users with admin rights close the review request, the email template shows the wrong name in the signature at the bottom of the e-mail content.
		This change involves replacing the `review_request.submitter` with `user` to display the name of the admin / user that closed the review request instead of the owner of the review request.
	+
	+	Things to clarify:
	+	- Please confirm if I need to also change the variable in this html template.I would like to confirm if it is sufficient to include screenshots that show the change.
	+
	+	Screenshot3 shows the html markdown version that is created when the admin closed the review request. I think the name in the `By` line should be superUser Admin. However, I ran into errors when I tried to change the variable `review_request.submitter` to `user` in the HTML template. I would like to confirm if it is required to change the variable in the HTML template. If I need to change that, then I would need some guidance of how to fix the errors that I showed in the 4th screenshot. The template tags did not cause any problem before I changed the variable so I was not sure what exactly was the problem and how to fix that.

Testing Done:

		I am not sure if unit tests are needed for this fix.
		I have included the screenshots that show the changes:

		This screenshot shows the e-mail that is created when I re-opened the review request and published it. My name is in the signature of the e-email.
		This screenshot shows the e-mail that is created when a user with admin rights closed the review request (discarded). The name of the user with admin rights (superUser Admin) is in the signature of the e-mail.
-
-		Screenshot 3 (require confirmation and clarification)
-		This screenshot shows the html markdown version that is created when the admin closed the review request. I think the name in the `By` line should be superUser Admin. However, I ran into errors when I tried to change the variable in the template to `user`. Please confirm if I need to also change the variable in this html template.

Added Files:

Change Summary:

Added the broken review request html file

Added Files:

Commits:

Summary	ID	Author
Fixed the signature in the email sent when the review request is closed by admin	7ccae188fe806fc26f29885f5edc2f085926f782	sng06
Fixed the signature in the email sent when the review request is closed by admin	7ccae188fe806fc26f29885f5edc2f085926f782	sng06
Changed the review_request.submitter variable to user	e9691727a6dfeccbde6dcd8e806737f7851ca2c9	sng06

Diff:

Revision 2 (+288 -262)

Show changes

	reviewboard/templates/notifications/review_request_email.txt
	reviewboard/templates/notifications/review_request_email.html

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Reverted changes in the html file, disabled VScode auto-formatting, changed the variable and fixed the error

Description:

		Currently, when users with admin rights close the review request, the email template shows the wrong name in the signature at the bottom of the e-mail content.
		This change involves replacing the `review_request.submitter` with `user` to display the name of the admin / user that closed the review request instead of the owner of the review request.
-
-		Things to clarify:
-		- Please confirm if I need to also change the variable in this html template.I would like to confirm if it is sufficient to include screenshots that show the change.
-
-		Screenshot3 shows the html markdown version that is created when the admin closed the review request. I think the name in the `By` line should be superUser Admin. However, I ran into errors when I tried to change the variable `review_request.submitter` to `user` in the HTML template. I would like to confirm if it is required to change the variable in the HTML template. If I need to change that, then I would need some guidance of how to fix the errors that I showed in the 4th screenshot. The template tags did not cause any problem before I changed the variable so I was not sure what exactly was the problem and how to fix that.

Testing Done:

		I am not sure if unit tests are needed for this fix.
		I have included the screenshots that show the changes:

~		This screenshot shows the e-mail that is created when I re-opened the review request and published it. My name is in the signature of the e-email.
~		This screenshot shows the e-mail that is created when a user with admin rights closed the review request (discarded). The name of the user with admin rights (superUser Admin) is in the signature of the e-mail.
	~	This screenshot shows the `TXT` e-mail that is created when I re-opened the review request and published it. My name is in the signature of the e-email.
	~	This screenshot shows the `TXT` e-mail that is created when a user with admin rights closed the review request (discarded). The name of the user with admin rights (superUser Admin) is in the signature of the e-mail.
	+	This screenshot shows the "Before" view of the `HTML` close e-mail. My name as the review request submitter is shown in the `By` line.
	+	This screenshot shows the "After" view of the `HTML` close e-mail. It has changed to the name of the user (superUser Admin) who closed the review request.

Commits:

Summary	ID	Author
Fixed the signature in the email sent when the review request is closed by admin	7ccae188fe806fc26f29885f5edc2f085926f782	sng06
Changed the review_request.submitter variable to user	e9691727a6dfeccbde6dcd8e806737f7851ca2c9	sng06
Fixed the signature in the email sent when the review request is closed by admin	7ccae188fe806fc26f29885f5edc2f085926f782	sng06
Changed the review_request.submitter variable to user	e9691727a6dfeccbde6dcd8e806737f7851ca2c9	sng06
Reverted changes in html file and changed the signature variable to user	a3ee42c2d58bc73f0132c6214e48d499965b0f34	sng06

Diff:

Revision 3 (+276 -276)

Show changes

	reviewboard/templates/notifications/review_request_email.txt
	reviewboard/templates/notifications/review_request_email.html

Removed Files:

Added Files:

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Summary:

-	[WIP] Fixed the signature in the email sent when the review request is closed by admin
+	Fixed the signature in the email sent when the review request is closed by admin

Description:

~		Currently, when users with admin rights close the review request, the email template shows the wrong name in the signature at the bottom of the e-mail content.
~		This change involves replacing the `review_request.submitter` with `user` to display the name of the admin / user that closed the review request instead of the owner of the review request.
	~	Currently, when users with admin rights close the review request,
	~	the email template shows the wrong name in the signature at the
	+	bottom of the e-mail content.
	+
	+	This change involves replacing the `review_request.submitter`
	+	with `user` to display the name of the admin / user that
	+	closed the review request instead of the owner of the
	+	review request.

Testing Done:

~		I am not sure if unit tests are needed for this fix.
	~	Added unit test to `test_sending_email.py`
-		I have included the screenshots that show the changes:

~		This screenshot shows the `TXT` e-mail that is created when I re-opened the review request and published it. My name is in the signature of the e-email.
~		This screenshot shows the `TXT` e-mail that is created when a user with admin rights closed the review request (discarded). The name of the user with admin rights (superUser Admin) is in the signature of the e-mail.
~		This screenshot shows the "Before" view of the `HTML` close e-mail. My name as the review request submitter is shown in the `By` line.
~		This screenshot shows the "After" view of the `HTML` close e-mail. It has changed to the name of the user (superUser Admin) who closed the review request.
	~	I have also included the screenshots that show the changes:
	~
	~	This screenshot shows the `TXT` e-mail that is created when I re-opened the review request and published it. My name is in the signature of the e-email.
	~	This screenshot shows the `TXT` e-mail that is created when a user with admin rights closed the review request (discarded). The name of the user with admin rights (superUser Admin) is in the signature of the e-mail.

Commits:

Summary	ID	Author
Fixed the signature in the email sent when the review request is closed by admin	7ccae188fe806fc26f29885f5edc2f085926f782	sng06
Changed the review_request.submitter variable to user	e9691727a6dfeccbde6dcd8e806737f7851ca2c9	sng06
Reverted changes in html file and changed the signature variable to user	a3ee42c2d58bc73f0132c6214e48d499965b0f34	sng06
Fixed the signature in the email sent when the review request is closed by admin	62e9427cc4bd71357e8aac03bad7aa1e4bd866c9	sng06
Changed the review_request.submitter variable to user	87cc93be33759e98125ddd9e17be290f9acf87f3	sng06
Reverted changes in html file and changed the signature variable to user	90c8a70195d1536172ff9c6cdd7b5cf34c1e97be	sng06
Reverted changes in html template as we want to preserve the author in the By field	d14924e1acb54edafefcb5f579de1f9bb7bc6565	sng06
Added unit test to test changes of close email after fixing the signature	0263c2d9973b9306f31200a8c26ce41c865b17fd	sng06

Diff:

Revision 4 (+340 -276)

Show changes

	reviewboard/notifications/tests/test_email_sending.py
	reviewboard/templates/notifications/review_request_email.txt

Removed Files:

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/notifications/tests/test_email_sending.py (Diff revision 4)

The issue has been resolved. Show all issues

May be a minor thing, but you can shorten this, by including publish=True inside the self.create_review_request function.

reviewboard/notifications/tests/test_email_sending.py (Diff revision 4)
The issue has been resolved. Show all issues
```
I know some of the other things in here say "Tests", but all test docstrings should start with "Testing"
```

reviewboard/notifications/tests/test_email_sending.py (Diff revision 4)

The issue has been resolved. Show all issues

These can be self.assertIn('...', message.as_string()), which I think will give us better debug output if they fail.

Change Summary:

Made changes to codes based on code review feedback

Commits:

Summary	ID	Author
Fixed the signature in the email sent when the review request is closed by admin	62e9427cc4bd71357e8aac03bad7aa1e4bd866c9	sng06
Changed the review_request.submitter variable to user	87cc93be33759e98125ddd9e17be290f9acf87f3	sng06
Reverted changes in html file and changed the signature variable to user	90c8a70195d1536172ff9c6cdd7b5cf34c1e97be	sng06
Reverted changes in html template as we want to preserve the author in the By field	d14924e1acb54edafefcb5f579de1f9bb7bc6565	sng06
Added unit test to test changes of close email after fixing the signature	0263c2d9973b9306f31200a8c26ce41c865b17fd	sng06
Fixed the signature in the email sent when the review request is closed by admin	f9a5d2799004930b7c5220acd404165e69f34170	sng06
Changed the review_request.submitter variable to user	737990f499cd7d73fab539744ea94a8479dedce2	sng06
Reverted changes in html file and changed the signature variable to user	73d71b7666cebc7c00c42c4a69375b12ccad5457	sng06
Reverted changes in html template as we want to preserve the author in the By field	e7e5225b43aa7d152b71b585fc8dc934320ff620	sng06
Added unit test to test changes of close email after fixing the signature	515b361b5fe1ddbae29ce5a991e0a889d199707f	sng06
Made changes to codes according to code review feedback received	9c3ba0baa111c96e7361337fdfee191a2145475f	sng06

Diff:

Revision 5 (+345 -283)

Show changes

	reviewboard/notifications/tests/test_email_sending.py
	reviewboard/templates/notifications/review_request_email.txt

Checks run (2 succeeded)

flake8 passed.

JSHint passed.