Link Sharing / New User Management

[feliks-montez]

Overview

This is a large addition that replaces the user-management-app module, now called the share-with-others module. It adds the additional feature of allowing project owners to invite new members via a reusable invite link. All user/permissions management has been moved to a single modal that can be opened from anywhere within a lexicon project. All permissions/members changes are updated in real time.

See the trello card.

Current UI

Desktop

Mobile

Frontend features

• Add ProjectRole, ProjectRoles, and LexRoles typings to reflect project roles on the backend in ProjectRoles.php and LexRoles.php.
• Adds ProjectService methods to correspond to the API calls added in Back end for link sharing #759.

UI Features

The following features are implemented but not clearly understood from the screenshot.

• After typing in an email address and clicking send, the proper callbacks fire to invite the user via an email and assigns them the permission level specified by the dropdown.
• The Reusable Invite Link feature works and is appropriately tied to the backend inviteToken methods. Anyone who accesses link will be added to the project as a member under the role specified by the dropdown.
• Clicking on one of the disabled link textboxes auto-selects the URL for easy copying.
• The Filter Members box is functional and has an X that can be clicked to clear it.
• When the Anyone (no-sign-in-required) is disabled, it is greyed out and does not automatically highlight the URL when clicked. Otherwise, it looks and behaves normally.
• The user list is populated from the DB.
• The Xs remove members from the project.
• Share buttons have been added; one in right of the navbar and one replaces the old User Management button in the cog dropdown.
• The dropdowns next to members allow changing of the member's role in the project.
• Allow others to share checkbox at the bottom allows non-manager members to access a limited version of the sharing dialog:
  

Backend Features

• sendInvite now allows specification of which role to assign to the invited user.
• sendInvite now checks to make sure the invitingUser has authority to invite based on project.allowSharing and its own role authority.
• useInviteToken from Back end for link sharing #759 now checks to see if the user accessing the invite link is already a member of the project. Before, if you accessed it as the owner, you could accidentally demote yourself.

Yet Unimplemented

• Does not display/allow accepting of join requests.
• Anyone, no-sign-in-required feature
• List publicly on languageforge.org checkbox at bottom of modal

Issues

• The following error is displayed every time the modal loads:
  

• Depending on phone size, it could be hard to tell that the members list scrolls.

• Done button may be off-screen for mobile.

• Uses LexRole in UserCommands.php, which is not an exclusively LF file. The code is written in such a way so as not to interfere with SF, but present nonetheless.

• I've noticed that ProjectCommands::updateUserRole allows updating to even non-existent roles.

---

This change is 

[megahirt]

This is looking really great! I have a number of comments below. This PR will become the main feature PR including changes to the PHP back-end.

Reviewed 21 of 21 files at r1.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @feliks-montez)

---

src/angular-app/bellows/shared/model/user.model.ts, line 1 at r1 (raw file):

export class User {

in a future commit, we will need to ensure that the TS models follow the PHP models exactly. Fine that for now this is stubbed out.

---

src/angular-app/languageforge/lexicon/lexicon.scss, line 1 at r1 (raw file):

@import 'editor/editor';

It looks to me like all of these additions are specific to the modal. Let's move these out of lexicon.scss and put them in /share-with-others/share-with-others.scss

---

src/angular-app/languageforge/lexicon/lexicon.scss, line 6 at r1 (raw file):

@import '../../../angular-app/bellows/apps/activity/activity.scss';

.inline-form {

.inline-form seems to me like a generic class that you could be affecting the CSS of other inputs/forms in the application and you wouldn't know it. I'm guessing this query selector should be made more specific, perhaps with another class name or id ?

---

src/angular-app/languageforge/lexicon/lexicon-app.component.html, line 9 at r1 (raw file):

    <div data-ui-view></div>

    <i class="fa fa-share-alt" data-ng-click="$ctrl.openShareWithOthersModal()"></i>

I can't see from the screenshot where the Share button is located. Can you post one? Also, we should change the menu item text link from "User Management" to "Share With Others" and have it launch the modal, would you agree?

---

src/angular-app/languageforge/lexicon/shared/share-with-others/_permissions-dropdown.scss, line 0 at r1 (raw file):
Delete this file please.

---

src/angular-app/languageforge/lexicon/shared/share-with-others/share-with-others.modal.html, line 7 at r1 (raw file):

  </div>
  <div class="modal-body">
    <user-management-app></user-management-app>

Per our conversation, you can keep <user-management> and move the non-user functions up to the modal component level (this one)

---

src/angular-app/languageforge/lexicon/shared/share-with-others/user-management-app.component.ts, line 65 at r1 (raw file):

  }

  queryUserList(): angular.IPromise<any> {

I would like to refactor/rename this method at some point. I never liked the 'queryUserList` name.

[megahirt]

I also note that we talked about UI layout changes for the mobile version, since that is not currently displaying properly

Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @feliks-montez)

[megahirt]

Draft: Share With Others UI

[feliks-montez]

Reviewable status: 8 of 25 files reviewed, 7 unresolved discussions (waiting on @feliks-montez and @megahirt)

---

src/angular-app/languageforge/lexicon/lexicon.scss, line 1 at r1 (raw file):

Previously, megahirt (Christopher Hirt) wrote…

It looks to me like all of these additions are specific to the modal. Let's move these out of lexicon.scss and put them in /share-with-others/share-with-others.scss

Done.

---

src/angular-app/languageforge/lexicon/lexicon.scss, line 6 at r1 (raw file):

Previously, megahirt (Christopher Hirt) wrote…

.inline-form seems to me like a generic class that you could be affecting the CSS of other inputs/forms in the application and you wouldn't know it. I'm guessing this query selector should be made more specific, perhaps with another class name or id ?

Done.

---

src/angular-app/languageforge/lexicon/lexicon-app.component.html, line 9 at r1 (raw file):

Previously, megahirt (Christopher Hirt) wrote…

I can't see from the screenshot where the Share button is located. Can you post one? Also, we should change the menu item text link from "User Management" to "Share With Others" and have it launch the modal, would you agree?

Done.

---

src/angular-app/languageforge/lexicon/shared/share-with-others/_permissions-dropdown.scss, line at r1 (raw file):

Previously, megahirt (Christopher Hirt) wrote…

Delete this file please.

Done.

---

src/angular-app/languageforge/lexicon/shared/share-with-others/share-with-others.modal.html, line 7 at r1 (raw file):

Previously, megahirt (Christopher Hirt) wrote…

Per our conversation, you can keep <user-management> and move the non-user functions up to the modal component level (this one)

Done. After talking with Ira, I feel pretty good about the current layout of this. If you have more thoughts, let me know.

---

src/angular-app/languageforge/lexicon/shared/share-with-others/user-management-app.component.ts, line 65 at r1 (raw file):

Previously, megahirt (Christopher Hirt) wrote…

I would like to refactor/rename this method at some point. I never liked the 'queryUserList` name.

Done.

[megahirt]

Looks good so far! Let's keep this PR as the one that will integrate the server-side

Reviewed 20 of 25 files at r2.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @feliks-montez)

---

src/angular-app/bellows/shared/model/user.model.ts, line 10 at r2 (raw file):

  avatar_ref?: string;
  avatarUrl?: string;
  isInvitee?: boolean;

The user management app has specific code that helps determine if an user is an "invitee" or not. Check out this user management code and I think you'll find that we don't need the invitee flag on the user. At least that's what my memory says.

---

src/angular-app/languageforge/lexicon/shared/share-with-others/permissions-dropdown.component.html, line 15 at r2 (raw file):

    <div class="dropdown-divider" ng-if="$ctrl.allowDelete"></div>
    <li class="dropdown-item" role="menuitem" ng-if="$ctrl.allowDelete">
      <div ng-click="$ctrl.onDeleteTarget({$event: {target: $ctrl.target}})" class="text-danger">remove member from project</div>

How about just "Remove"?

---

src/angular-app/languageforge/lexicon/shared/share-with-others/share-with-others.modal.html, line 7 at r1 (raw file):

Previously, feliks-montez (Feliks Montez) wrote…

Done. After talking with Ira, I feel pretty good about the current layout of this. If you have more thoughts, let me know.

ok sounds good.

---

src/Site/views/languageforge/theme/default/sass/_global.scss, line 44 at r2 (raw file):

  }

  #shareBtn {

you defined #shareBtn twice in this file. Let's keep it under a single definition.

---

src/Site/views/languageforge/theme/default/sass/_global.scss, line 272 at r2 (raw file):

  }

  #shareBtn {

you defined #shareBtn twice in this file. Let's keep it under a single definition.

[irahopkinson]

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @feliks-montez and @megahirt)

---

src/angular-app/bellows/shared/model/user.model.ts, line 10 at r2 (raw file):

Previously, megahirt (Christopher Hirt) wrote…

The user management app has specific code that helps determine if an user is an "invitee" or not. Check out this user management code and I think you'll find that we don't need the invitee flag on the user. At least that's what my memory says.

Nope. At conference earlier this year we fixed a nasty security bug because we did not have the isInvitee flag and different bits of the code were inferring that status different ways. We migrated all existing data so the flag is now the authoritative and correct way to check this status.

[feliks-montez]

Reviewable status: 8 of 37 files reviewed, 5 unresolved discussions (waiting on @feliks-montez and @megahirt)

---

src/Site/views/languageforge/theme/default/sass/_global.scss, line 44 at r2 (raw file):

Previously, megahirt (Christopher Hirt) wrote…

you defined #shareBtn twice in this file. Let's keep it under a single definition.

One of them is under a @media query for mobile.

---

src/Site/views/languageforge/theme/default/sass/_global.scss, line 272 at r2 (raw file):

Previously, megahirt (Christopher Hirt) wrote…

you defined #shareBtn twice in this file. Let's keep it under a single definition.

Same as above.

---

src/angular-app/languageforge/lexicon/shared/share-with-others/permissions-dropdown.component.html, line 15 at r2 (raw file):

Previously, megahirt (Christopher Hirt) wrote…

How about just "Remove"?

Done.

[megahirt]

Great work @feliks-montez @Andrewdt97 @Nateowami ! This is being merged :) There are several todos which @Nateowami has called out in his previous PR that we'll look to tighten up post-merge. Thanks all!

Reviewed 25 of 31 files at r3, 5 of 5 files at r4.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @feliks-montez)

---

src/Site/views/languageforge/theme/default/sass/_global.scss, line 44 at r2 (raw file):

Previously, feliks-montez (Feliks Montez) wrote…

One of them is under a @media query for mobile.

ok got it, thanks.

---

src/Site/views/languageforge/theme/default/sass/_global.scss, line 272 at r2 (raw file):

Previously, feliks-montez (Feliks Montez) wrote…

Same as above.

ok I see that now. Thanks.

[megahirt]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @feliks-montez)

[megahirt]

Reviewable status: complete! all files reviewed, all discussions resolved