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	Author
Fixed the signature in the email sent when the review request is closed by admin	sng06
Changed the review_request.submitter variable to user	sng06
Reverted changes in html file and changed the signature variable to user	sng06
Reverted changes in html template as we want to preserve the author in the By field	sng06
Added unit test to test changes of close email after fixing the signature	sng06
Made changes to codes according to code review feedback received	sng06

Screenshots

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

Author

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

sng06

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

sng06

Changed the review_request.submitter variable to user

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	Author
-	Fixed the signature in the email sent when the review request is closed by admin	sng06
-	Changed the review_request.submitter variable to user	sng06
+	Fixed the signature in the email sent when the review request is closed by admin	sng06
+	Changed the review_request.submitter variable to user	sng06
+	Reverted changes in html file and changed the signature variable to user	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	Author
-	Fixed the signature in the email sent when the review request is closed by admin	sng06
-	Changed the review_request.submitter variable to user	sng06
-	Reverted changes in html file and changed the signature variable to user	sng06
+	Fixed the signature in the email sent when the review request is closed by admin	sng06
+	Changed the review_request.submitter variable to user	sng06
+	Reverted changes in html file and changed the signature variable to user	sng06
+	Reverted changes in html template as we want to preserve the author in the By field	sng06
+	Added unit test to test changes of close email after fixing the signature	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)

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)

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)

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	Author
-	Fixed the signature in the email sent when the review request is closed by admin	sng06
-	Changed the review_request.submitter variable to user	sng06
-	Reverted changes in html file and changed the signature variable to user	sng06
-	Reverted changes in html template as we want to preserve the author in the By field	sng06
-	Added unit test to test changes of close email after fixing the signature	sng06
+	Fixed the signature in the email sent when the review request is closed by admin	sng06
+	Changed the review_request.submitter variable to user	sng06
+	Reverted changes in html file and changed the signature variable to user	sng06
+	Reverted changes in html template as we want to preserve the author in the By field	sng06
+	Added unit test to test changes of close email after fixing the signature	sng06
+	Made changes to codes according to code review feedback received	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.