"This field may not be null." when trying to update my user profile (in -prod)

[willdurand]

For some reasons, I cannot update my user profile in production. I believe there is some sort of permission issue but I am not sure. I only see this:

and the API response contains:

{"reviewer_name":["This field may not be null."]}

Note: there is no "reviewer name" field on this page.

[willdurand]

@eviljeff or @mat do users need to set a "reviewer name" when they have the permission ReviewerTools:View? (because that's what I have in -prod)

[willdurand]

So to answer my own question, the serializer uses this function to show/hide the reviewer_name: https://github.com/mozilla/addons-server/blob/647a0f09b893bff7ffe4140cbee7e319d65de3e5/src/olympia/access/acl.py#L154-L173

This is not exactly what we have currently in addons-frontend:

addons-frontend/src/amo/reducers/users.js

Lines 488 to 515 in a956af9

	export const hasAnyReviewerRelatedPermission = (state: AppState): boolean => {
	const currentUser = getCurrentUser(state.users);
	// If the user isn't authenticated, they have no permissions.
	if (!currentUser) {
	return false;
	}
	const { permissions } = currentUser;
	if (!permissions) {
	return false;
	}
	// Admins have absolutely all permissions.
	if (permissions.includes(ALL_SUPER_POWERS)) {
	return true;
	}
	return (
	permissions.includes(ADDONS_POSTREVIEW) ||
	permissions.includes(ADDONS_CONTENTREVIEW) ||
	permissions.includes(ADDONS_REVIEW) ||
	permissions.includes(RATINGS_MODERATE) ||
	permissions.includes(THEMES_REVIEW) ||
	permissions.includes(ADDONS_REVIEWUNLISTED) ||
	permissions.includes(ADDONS_EDIT)
	);
	};

[eviljeff]

do users need to set a "reviewer name" when they have the permission ReviewerTools:View? (because that's what I have in -prod)

No, it's optional. (allow_blank=True)

So to answer my own question, the serializer uses this function to show/hide the reviewer_name: https://github.com/mozilla/addons-server/blob/647a0f09b893bff7ffe4140cbee7e319d65de3e5/src/olympia/access/acl.py#L154-L173

that doesn't seem to be anything to do with reviewer_name

[willdurand]

that doesn't seem to be anything to do with reviewer_name

mm, what about this then? https://github.com/mozilla/addons-server/blob/855464d3890d858edf9b9a6cf09dec14c7f8666a/src/olympia/accounts/serializers.py#L98-L102

[willdurand]

No, it's optional. (allow_blank=True)

Does blank allow null? I get that it can be empty (empty string) but it is likely None/null in my case, hence the error.

[eviljeff]

mm, what about this then? https://github.com/mozilla/addons-server/blob/855464d3890d858edf9b9a6cf09dec14c7f8666a/src/olympia/accounts/serializers.py#L98-L102

yeah, it's easier to drop a serializer field if you don't want it in the response than to dynamically add one. We only want it for reviewers.

[eviljeff]

Does blank allow null? I get that it can be empty (empty string) but it is likely None/null in my case, hence the error.

The model allows None (as well as blank):
https://github.com/mozilla/addons-server/blob/master/src/olympia/users/models.py#L173:L175

But the serializer doesn't have allow_null, only allow_blank so I suspect it would error if null was sent:
https://github.com/mozilla/addons-server/blob/master/src/olympia/accounts/serializers.py#L80:L82

[willdurand]

ok so that explains why it failed for me. We likely send null instead of an empty string.

Should we change anything in addons-server? (like adding allow_null to the serializer?)

[eviljeff]

we could... though why would addons-frontend send null instead of an empty string? The form input value is "" isn't it?

[willdurand]

we could... though why would addons-frontend send null instead of an empty string? The form input value is "" isn't it?

In that case, there was no field at all because of a permission problem. The permission shows a field and the field will set the value to an empty string.

[eviljeff]

So it's a problem that can't happen any more?

[willdurand]

So it's a problem that can't happen any more?

I think so, yes.

[ioanarusiczki]

@willdurand I am not sure I understand it completely. I should see if a user with theme permissions mentioned here will not receive the red error message while updating the profile?

[willdurand]

@willdurand I am not sure I understand it completely. I should see if a user with theme permissions mentioned here will not receive the red error message while updating the profile?

A user with ReviewerTools:View and no reviewer name set should be able to update their profile on AMO. This isn't the case in production because we hide the reviewer name field (hence the error message mentioned above).

[ioanarusiczki]

@willdurand The user I added in the Reviewers:read only group from admin tools dev which has the ReviewerTools:View permission does not have on the edit profile page a reviewer name field.
I added or removed info from the profile and I did not receive any errors.

Verified on dev with FF74(Win10).

[willdurand]

mmm, @wagnerand do you know which permissions my user in production has?

[ioanarusiczki]

@willdurand That would be great if we could find out the info from @wagnerand so I could do a correct check.

[wagnerand]

@willdurand @ioanarusiczki I sent you both a DM with the details.

[ioanarusiczki]

@willdurand With the given permissions I could not reproduce the issue on dev. This type of user doesn't have a rev tool name field to fill in edit profile page.

[willdurand]

ha, well I guess I'll check in prod then

[willdurand]

@ioanarusiczki FWIW, my user has reviewer_name set to null in the API response (to fetch user information). I don't know how this is possible but if you manage to have a user with reviewer_name: null and the same permissions mentioned above, then you should be able to reproduce.

[ioanarusiczki]

@willdurand Alright! I'm going to take a look at this again.

[willdurand]

I verified in prod and it worked for me :)