Summary

Fix My Privacy Rights showing empty label.

Review Request #11180 — Created Sept. 18, 2020 and submitted Oct. 28, 2020, 11:07 a.m.

Information

Owner

MarcusBoay

Repository

Review Board

Branch

release-4.0.x

Bugs

4882

Depends On

Reviewers

Groups

reviewboard

People

Description

The "My Privacy Rights" page in the user account settings was showing an
orphaned :*. This was for the field label for the consent items, but that
form field didn't have a label defined. The cause here was that the Review
Board form field template was checking if the form had a fields_no_label
attribute, but that was deprecated and removed from Djblets a long time ago.
The correct fix is to check if the field has a label defined.

Testing Done

Manual testing by refreshing the My Privacy Rights page and checking that the
empty label is no longer visible. Other labels on the account forms that are
supposed to be there have been verified to be visible.

Doing a search of fields_no_label on all the reviewboard repositories show
no usage of that particular variable.

Commits

Summary	ID
Fix My Privacy Rights showing empty label.	65294bd755f533c49e4b8fc5ffac7cfb424b455c

Issues

Description	From	Last Updated
Your description is a bit too deep in the weeds. You don't have to tell us what line you changed …	david	Sept. 29, 2020, 7:52 p.m.

flake8 passed.

JSHint passed.

Description:: Fix My Privacy Rights showing empty label.

~
Simply changing not form.fields_no_label to field.label on line 25 of reviewboard/templates/forms/field.html. Doing a search of fields_no_label on all the reviewboard repo show no usage of that particular variable.
~
Simply changing not form.fields_no_label to field.label on line 25 of
+ reviewboard/templates/forms/field.html. Doing a search of fields_no_label
+ on all the reviewboard repo show no usage of that particular variable.

~
However, there is are two references to the fields_no_label variable in the djblets repo. One in docs/releasenotes/0.6.14.rst and another in /djblets/forms/templates/djblets_forms/form_field.html. There are no code references.
~
However, there is are two references to the fields_no_label variable in the
+ djblets repo. One in docs/releasenotes/0.6.14.rst and another in
+ /djblets/forms/templates/djblets_forms/form_field.html. There are no code
+ references.
Testing Done:: ~
Manual testing by refreshing the My Privacy Rights page and checking that the empty label is no longer visible.
~
Manual testing by refreshing the My Privacy Rights page and checking that the
+ empty label is no longer visible.

Description:: Fix My Privacy Rights showing empty label.

Simply changing not form.fields_no_label to field.label on line 25 of
reviewboard/templates/forms/field.html. Doing a search of fields_no_label
~ on all the reviewboard repo show no usage of that particular variable.
~ on all the reviewboard repositories show no usage of that particular
+ variable.

However, there is are two references to the fields_no_label variable in the
djblets repo. One in docs/releasenotes/0.6.14.rst and another in
/djblets/forms/templates/djblets_forms/form_field.html. There are no code
references.

The issue has been resolved. Show all issues

Your description is a bit too deep in the weeds. You don't have to tell us what line you changed the code on, or what the code used to say and what it says now--that's what the diff is for.

In general, for bug fixes, we like to describe what the bug was, what was wrong, and how we fixed it. So something like:

The "My Privacy Rights" page in the user account settings was showing an orphaned ":*". This was for the field label for the consent items, but that form field didn't have a label defined.
The cause here was that the Review Board form field template was checking if the form had a fields_no_label attribute, but that was deprecated and removed from Djblets a long time ago. The correct fix is to check if the field has a label defined.

You can mention that you grepped around for fields_no_label in the testing done section rather than the description. It might be nice to also hear that you looked through some of the other account forms and verified that all the labels that are supposed to be visible still are.

MarcusBoay Sept. 29, 2020, 7:52 p.m.

Thanks for the feedback! I updated the description and testing done sections accordingly. Let me know if there is anything else.

Description:: ~
Fix My Privacy Rights showing empty label.
~
~
Simply changing not form.fields_no_label to field.label on line 25 of
~ reviewboard/templates/forms/field.html. Doing a search of fields_no_label
~ on all the reviewboard repositories show no usage of that particular
~ variable.
~
The "My Privacy Rights" page in the user account settings was showing an
~ orphaned :*. This was for the field label for the consent items, but that
~ form field didn't have a label defined. The cause here was that the Review
~ Board form field template was checking if the form had a fields_no_label
~ attribute, but that was deprecated and removed from Djblets a long time ago.
~ The correct fix is to check if the field has a label defined.
-
-
However, there is are two references to the fields_no_label variable in the
- djblets repo. One in docs/releasenotes/0.6.14.rst and another in
- /djblets/forms/templates/djblets_forms/form_field.html. There are no code
- references.
Testing Done:: Manual testing by refreshing the My Privacy Rights page and checking that the
~ empty label is no longer visible.
~ empty label is no longer visible. Other labels on the account forms that are
+ supposed to be there have been verified to be visible.
+
+
Doing a search of fields_no_label on all the reviewboard repositories show
+ no usage of that particular variable.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-4.0.x (55b5664)

		Fix My Privacy Rights showing empty label.

~		Simply changing `not form.fields_no_label` to `field.label` on line 25 of `reviewboard/templates/forms/field.html`. Doing a search of `fields_no_label` on all the reviewboard repo show no usage of that particular variable.
	~	Simply changing `not form.fields_no_label` to `field.label` on line 25 of
	+	`reviewboard/templates/forms/field.html`. Doing a search of `fields_no_label`
	+	on all the reviewboard repo show no usage of that particular variable.

~		However, there is are two references to the `fields_no_label` variable in the djblets repo. One in `docs/releasenotes/0.6.14.rst` and another in `/djblets/forms/templates/djblets_forms/form_field.html`. There are no code references.
	~	However, there is are two references to the `fields_no_label` variable in the
	+	djblets repo. One in `docs/releasenotes/0.6.14.rst` and another in
	+	`/djblets/forms/templates/djblets_forms/form_field.html`. There are no code
	+	references.

~		Manual testing by refreshing the My Privacy Rights page and checking that the empty label is no longer visible.
	~	Manual testing by refreshing the My Privacy Rights page and checking that the
	+	empty label is no longer visible.

		Manual testing by refreshing the My Privacy Rights page and checking that the
~		empty label is no longer visible.
	~	empty label is no longer visible. Other labels on the account forms that are
	+	supposed to be there have been verified to be visible.
	+
	+	Doing a search of `fields_no_label` on all the reviewboard repositories show
	+	no usage of that particular variable.