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): add Organization Settings route TASK-981 #5299

Merged
merged 31 commits into from
Dec 5, 2024

Conversation

magicznyleszek
Copy link
Member

@magicznyleszek magicznyleszek commented Nov 25, 2024

📣 Summary

In Organization Settings route (#/account/organization/settings) display editable organization name, and website. Also display non-editable organization type.

📖 Description

Only some roles allow editing the fields, and there is some logic in regards to fields being visible.

👀 Preview steps

  1. for one of the users (e.g. "joe"), use http://kf.kobo.local/admin/organizations/organization/ to enable "Multi-members override" (for "joe's organization")
  2. enable feature flag mmosEnabled
  3. navigate to #/account/organization/settings
  4. 🟢 notice that at least "Team name"/"Organization name" field is being displayed and is editable(-ish)
  • Editing the field doesn't do anything yet - this will be implemented in next PR
  1. 🟢 if you have enabled Stripe - notice that "Team website"/"Organization website" field and "Team type"/"Organzation type" (non editable) fields are also being displayed

Next steps:

  1. for another user (e.g. "zoe"), use http://kf.kobo.local/admin/organizations/organization/ to add "zoe" into "joe's organization"
  2. logged in as "zoe", navigate to #/account/organization/settings
  3. 🟢 notice that at least "Team name"/"Organization name" field is being displayed (non editable)
  4. 🟢 if you have enabled Stripe - notice that "Team website"/"Organization website" field and "Team type"/"Organzation type" fields are also being displayed (both non editable)

@magicznyleszek magicznyleszek changed the title feaet(organizations): add Organization Settings route TASK-981 feat(organizations): add Organization Settings route TASK-981 Nov 26, 2024
@magicznyleszek magicznyleszek marked this pull request as ready for review November 26, 2024 19:33
@magicznyleszek magicznyleszek changed the base branch from main to leszek/task-1219-org-settings-mutating December 3, 2024 14:39
@magicznyleszek magicznyleszek requested review from pauloamorimbr and Akuukis and removed request for jnm, rgraber and noliveleger December 5, 2024 14:12
Copy link
Contributor

@Akuukis Akuukis left a comment

Choose a reason for hiding this comment

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

LGTM, not tested, see 2 nitpicks inline.

Comment on lines +83 to +89
let deletionMessage = t('To delete this ##team/org##, please contact the server administrator.')
.replaceAll('##team/org##', mmoLabelLowercase);
if (isStripeEnabled) {
deletionMessage = t("To delete this ##team/org##, you need to cancel your current ##plan name## plan. At the end of the plan period your ##team/org##'s projects will be converted to projects owned by your personal account.")
.replaceAll('##team/org##', mmoLabelLowercase)
.replace('##plan name##', subscriptions.planName);
}
Copy link
Contributor

@Akuukis Akuukis Dec 5, 2024

Choose a reason for hiding this comment

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

more of a linter thing, but don't use let and mutation if possible. Here one way to rewrite it

Suggested change
let deletionMessage = t('To delete this ##team/org##, please contact the server administrator.')
.replaceAll('##team/org##', mmoLabelLowercase);
if (isStripeEnabled) {
deletionMessage = t("To delete this ##team/org##, you need to cancel your current ##plan name## plan. At the end of the plan period your ##team/org##'s projects will be converted to projects owned by your personal account.")
.replaceAll('##team/org##', mmoLabelLowercase)
.replace('##plan name##', subscriptions.planName);
}
const deletionMessage = isStripeEnabled
? t("To delete this ##team/org##, you need to cancel your current ##plan name## plan. At the end of the plan period your ##team/org##'s projects will be converted to projects owned by your personal account.")
.replaceAll('##team/org##', mmoLabelLowercase)
.replace('##plan name##', subscriptions.planName)
: t('To delete this ##team/org##, please contact the server administrator.')
.replaceAll('##team/org##', mmoLabelLowercase);

or

  const deletionMessage = t(isStripeEnabled
    ? "To delete this ##team/org##, you need to cancel your current ##plan name## plan. At the end of the plan period your ##team/org##'s projects will be converted to projects owned by your personal account."
    : 'To delete this ##team/org##, please contact the server administrator.'
    )
      .replaceAll('##team/org##', mmoLabelLowercase)
      .replace('##plan name##', subscriptions.planName)

Copy link
Contributor

@pauloamorimbr pauloamorimbr left a comment

Choose a reason for hiding this comment

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

LGTM!
Tested and now it works like a charm! 👏🏻

Base automatically changed from leszek/task-1219-org-settings-mutating to main December 5, 2024 17:23
@magicznyleszek magicznyleszek merged commit f304269 into main Dec 5, 2024
7 checks passed
@magicznyleszek magicznyleszek deleted the kalvis/organizations-settings branch December 5, 2024 17:24
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.

3 participants