Skip to content

[Hold of Payment Sept 7] User can invite chronos@, receipts@ and concierge@ in Workspaces #4646

Closed

Description

@parasharrajat

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Open newDot.
  2. Go to any Workspace.
  3. Try to invite chronos@, receipts@, and concierge@.

Expected Result:

Users can not invite chronos@, receipts@, and concierge@ to the workspace.

Actual Result:

chronos@, receipts@, and concierge@ are added as workspace Users.

Workaround:

Don't invite chronos@, receipts@, and concierge@ to your workspace.

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.0.85.x
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
image

Expensify/Expensify Issue URL:

View all open jobs on Upwork

Proposal

  1. we have to exclude these system users while checking for input text validity for email and phone.
    const isEnteredLoginsvalid = _.every(logins, login => Str.isValidEmail(login) || Str.isValidPhone(login));
  2. we have these emails available in the CONST.js
    EMAIL: {
  3. We add the receipts@ to this const and create a general utility method that checks that is an entered email a system user isSystemUser.
  4. We can use that here and discard the logins with a Growl saying something System users are not allowed to be invited.

Activity

  1. MelvinBot commented on Aug 13, 2021

    @MelvinBot
    Contributor

    Triggered auto assignment to @trjExpensify (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

  2. removed
    AutoAssignerTriageAuto assign issues for triage to an available triage team member
    on Aug 13, 2021
  3. trjExpensify commented on Aug 16, 2021

    @trjExpensify
    Contributor

    Ah yes, we should definitely not allow this. We hide these users in the request money/split bills flows already. Handing over to engineering triage.

    CC: @MitchExpensify & @kevinksullivan as this is related to N6.

  4. MelvinBot commented on Aug 16, 2021

    @MelvinBot
    Contributor

    Triggered auto assignment to @roryabraham (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

  5. removed their assignment
    on Aug 16, 2021
  6. roryabraham commented on Aug 17, 2021

    @roryabraham
    Contributor

    @parasharrajat Can work on the front-end piece, but I think maybe we should duplicate this validation in the back-end. So this issue can be mixed internal and external. External in NewDot and internal in the API.

  7. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on Aug 17, 2021
  8. MelvinBot commented on Aug 17, 2021

    @MelvinBot
    Contributor

    Triggered auto assignment to @stephanieelliott (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

  9. 21 remaining items

  10. roryabraham commented on Aug 24, 2021

    @roryabraham
    Contributor

    Linked PR is now on HOLD for #4649 to be merged.

  11. Christinadobrzyn commented on Sep 1, 2021

    @Christinadobrzyn
    Contributor

    Looks like PR #4781 (comment) is merged! Holding for payment in 7 days.

    image

  12. changed the title [-]User can invite chronos@, receipts@ and concierge@ in Workspaces[/-] [+][Hold of Payment Sept 7] User can invite chronos@, receipts@ and concierge@ in Workspaces[/+] on Sep 1, 2021
  13. Santhosh-Sellavel commented on Sep 7, 2021

    @Santhosh-Sellavel
    Collaborator

    One clarification will I get a bonus for this since issue #4668 which I raised was closed. And we are proceeding with this one.

    @Santhosh-Sellavel I think you have a reasonable claim to that bonus, but I don't think this is the right issue to claim the bonus on. Your initial proposal was to include a hint in the input field and as this was added as a part of this issue I think you should raise this point there instead.

    As @Julesssss suggested bring up the question here!
    I see changes I proposed in #4668 were taken and implemented along with this issue.
    Will I get a bonus?
    Please clarify @Christinadobrzyn Thanks!

  14. parasharrajat commented on Sep 7, 2021

    @parasharrajat
    MemberAuthor

    I fixed 4 things in one. Should I also ask for bonus for extra issues? :) :--) But I don't want it as I fixed all issues that I found with inviting logic (this issue is linked with inviting part).

    1. Block system users from inviting into Workspace. (this issue).
    2. Fix a mistake where users can invite existing multiple users.
    3. Fix users can invite existing users with phone number login.
    4. Fix copy to show that multiple users can be invited. There is an option Invite multiple Users to workspace, but it's not conveyed to the user. #4668 was supposed to do this. I can see that it is closed and marked as a temporary issue. my solution is a little different than the proposed on the 4668. I have used multiline input and changed the copy above the input.
  15. Julesssss commented on Sep 7, 2021

    @Julesssss
    Contributor

    @Santhosh-Sellavel just to clarify, I didn't confirm that the changes you suggested were taken and implemented. I said that you have a good argument for it and that you should bring it up in this issue.

  16. Santhosh-Sellavel commented on Sep 7, 2021

    @Santhosh-Sellavel
    Collaborator

    @Julesssss Updated my comment accordingly, Since you pointed this issue out, I thought you were aware that changes I proposed were done in this issue. Sorry for the misunderstanding!

  17. Christinadobrzyn commented on Sep 8, 2021

    @Christinadobrzyn
    Contributor

    PR is merged for over 7 days - paid @parasharrajat for the job. Thanks! 🎉

    Upwork contract is closed. Closing this issue.

  18. Christinadobrzyn commented on Sep 8, 2021

    @Christinadobrzyn
    Contributor

    @Santhosh-Sellavel - thanks for raising the question about getting paid for this related issue. I will reach out to my team about this and follow up with you in issue #4668.

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

Metadata

Metadata

Labels

EngineeringExternalAdded to denote the issue can be worked on by a contributorImprovementItem broken or needs improvement.ReviewingHas a PR in reviewWeeklyKSv2

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions