Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disabled org admin role dropdown causes field error #823

Closed
linzjax opened this issue Oct 13, 2016 · 10 comments
Closed

Disabled org admin role dropdown causes field error #823

linzjax opened this issue Oct 13, 2016 · 10 comments
Assignees

Comments

@linzjax
Copy link
Contributor

linzjax commented Oct 13, 2016

Steps to reproduce the error

Go to an organization signed in as the organization administrator
Go to edit your member profile
Click save

Actual behavior

screen shot 2016-10-13 at 2 56 56 pm

### Expected behavior

Messaging that explains why I cannot edit my own profile.

@seav
Copy link
Contributor

seav commented Oct 13, 2016

Ah. Because of this report, I only learned now that disabled form fields are not submitted with the form. That is why you get the required error message. Apparently, the correct attribute should be readonly.

@oliverroick
Copy link
Member

I don't like the idea of disabling a field or making it read-only. For usability, wouldn't it be better not to display the role field and the remove button at all? Instead we can just display the role and use a hidden field for the admin role to prevent errors when submitting the form. @clash99, opinions?

Then there is something funny about these lines. org_admin is used in the template to work out whether the user can change the organization role or not. For superusers, the condition becomes False, therefore superusers can change the field’s value. After the form is submitted, an error is shown that they can’t change the role. Which one is correct, @wonderchook?

@clash99
Copy link
Contributor

clash99 commented Oct 25, 2016

Looking at this again I think a better interface might be to remove the gray box with the button and note completely. I would add a "?" next to the Role field and then @bethschechter can add help text containing this note and other role info. I think it'll make it simpler and less confusing.

Once we get the modals completed for the dropdown change, I also need to move that save button to be over the right side again. (More of a note-to-self)

@wonderchook
Copy link
Contributor

I think the superuser should be able to do anything. That is why there are
so few superusers.

On Tue, Oct 25, 2016 at 1:01 PM, Oliver Roick [email protected]
wrote:

I don't like the idea of disabling a field or making it read-only. For
usability, wouldn't it be better not to display the role field and the
remove button at all? Instead we can just display the role and use a hidden
field for the admin role to prevent errors when submitting the form.
@clash99 https://github.com/clash99, opinions?

Then there is something funny about these lines
https://github.com/Cadasta/cadasta-platform/blob/master/cadasta/organization/views/default.py#L247-L249.
org_admin is used in the template to work out whether the user can change
the organization role or not. For superusers, the condition becomes False,
therefore superusers can change the field’s value. After the form is
submitted, an error is shown that they can’t change the role. Which one is
correct, @wonderchook https://github.com/wonderchook?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#823 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEJmNEub69QCgbR0ffzsWDaUsstjxCsks5q3jYFgaJpZM4KWPkH
.

Kate Chapman
Chief Technology Officer // Cadasta
[email protected] /__/ +1 703 673 8834
cadasta.org // @CadastaOrg http://twitter.com/CadastaOrg

@linzjax
Copy link
Contributor Author

linzjax commented Jan 6, 2017

This issue has already been addressed. Should we close it?

@oliverroick
Copy link
Member

I think it was addressed when you reworked that area, @linzjax?!

@linzjax
Copy link
Contributor Author

linzjax commented Jan 6, 2017

Yeah... I just forgot about this issue... 😂

@clash99
Copy link
Contributor

clash99 commented Jan 6, 2017

screenshot 2017-01-06 09 42 10

It appears fixed. I'm going to add some css around the "Role Administratior" to it so it appears less confusing. I'll assign it to myself.

@clash99 clash99 reopened this Jan 6, 2017
@clash99 clash99 self-assigned this Jan 6, 2017
@clash99
Copy link
Contributor

clash99 commented Jan 6, 2017

Any idea on when we are going to get avatars into the platform? Should we hide this too? It does reinforce its a person (with a perfectly round head). Just a thought.

@clash99
Copy link
Contributor

clash99 commented Jan 6, 2017

Separated Administrator from Role for clarity:

screenshot 2017-01-06 09 49 15

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants