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

feat(organizations): disable email updates for regular organization members TASK-997 #5233

Merged
merged 11 commits into from
Nov 21, 2024

Conversation

duvld
Copy link
Member

@duvld duvld commented Nov 6, 2024

🗒️ Checklist

  1. run linter locally
  2. update all related docs (API, README, inline, etc.), if any
  3. draft PR with a title <type>(<scope>)<!>: <title> TASK-1234
  4. tag PR: at least frontend or backend unless it's global
  5. fill in the template below and delete template comments
  6. review thyself: read the diff and repro the preview as written
  7. open PR & confirm that CI passes
  8. request reviewers, if needed
  9. delete this section before merging

📣 Summary

UI for changing emails is disabled for Members of a multi-member organization.

👀 Preview steps

  1. Make sure your user's email is correct in both the user section of the django admin as well as the email address section. If there is a difference it would only show the former here. See https://www.notion.so/kobotoolbox/task-1236
  2. Make an MMO that has an owner, admin, and a member, as well as a regular user
  3. Navigate to the account settings page
  4. Visit the security section
  5. If the user is a regular user, or an MMO owner or admin, they should be able to change their email address as normal
  6. If the user is a member of an MMO the text box and button should be gone

📖 Description

Remove the text box and button entirely if the user is a member. Replaced with just text of the email returned from /me.

💭 Notes

Make sure the user either has the proper email reflected in the user section of the django admin or was created through an email confirmation link. See https://www.notion.so/kobotoolbox/task-1236

Copy link

@jamesrkiger jamesrkiger assigned duvld and unassigned jamesrkiger Nov 6, 2024
</>
)}
</div>
<div className={styles.options}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we move this container div inside the userCanChangeEmail check?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit of a hack, if the container div is inside of the check then the rest of the row CSS gets unaligned. The only way I saw to fix this was to add conditional styling which I thought looked worse than this. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I wondered if that might be the reason. I'd say we should go with conditional CSS. You're right that it will add some more complexity, but I think it will make it easier for other coders working on this section in the future to know what's going on.

Copy link
Contributor

@jamesrkiger jamesrkiger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@duvld There was an import error, so I pushed a fix for that. I also made a small change to simplify the logic in that cx() function. You can go ahead and merge unless you have any objections to my changes.

@duvld duvld merged commit 6729e75 into main Nov 21, 2024
6 of 7 checks passed
@duvld duvld deleted the task-997-disbable-email-if-member branch November 21, 2024 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants