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

[$250] BUG - Create room - Workspace is not selected by default if one workspace only available in the account #11650

Closed
kavimuru opened this issue Oct 6, 2022 · 29 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@kavimuru
Copy link

kavimuru commented Oct 6, 2022

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. Launch the app and login
  2. Click global fab button and select "New Room"
  3. Observe the workspace field

Expected Result:

When creating a new room, if you have only one workspace, it should be selected by default

Actual Result:

Select a workspace is shown as default and Showing an error telling the user to select a value from the list when there's only one value

Workaround:

Unknown

Platform:

Where is this issue occurring?

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

Version Number: 1.2.12-1
Reproducible in staging?: y
Reproducible in production?: y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:
image (1)
image

Recording.633.mp4

Expensify/Expensify Issue URL:
Issue reported by: @iwiznia
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1665065274983609

View all open jobs on GitHub

@kavimuru kavimuru added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels Oct 6, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 6, 2022

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

@melvin-bot melvin-bot bot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Oct 6, 2022
@rushatgabhane
Copy link
Member

rushatgabhane commented Oct 6, 2022

Been working on Picker a lot lately, might as well add a proposal for this.

Proposal

When the Picker has only one item, we should pass it using the value prop.

getValue() {
    if (isOnlyItem) return items[0].value
    return value || pickerValue
}

<Picker
    value={getValue}

value={this.props.value || this.pickerValue}

P.S. this.pickerValue is being removed in #11039, so it'll look prettier soon.

Screen.Recording.2022-10-07.at.3.16.17.AM.mov

@melvin-bot
Copy link

melvin-bot bot commented Oct 6, 2022

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

@alexpensify alexpensify removed their assignment Oct 6, 2022
@alexpensify alexpensify added Daily KSv2 and removed Weekly KSv2 labels Oct 6, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 10, 2022

@tgolen Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Oct 10, 2022
@tgolen
Copy link
Contributor

tgolen commented Oct 10, 2022

LGTM 👍 🟢

@melvin-bot melvin-bot bot removed the Overdue label Oct 10, 2022
@tgolen tgolen added the External Added to denote the issue can be worked on by a contributor label Oct 10, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 10, 2022

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

@melvin-bot
Copy link

melvin-bot bot commented Oct 10, 2022

Triggered auto assignment to Contributor-plus team member for initial proposal review - @sobitneupane (External)

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 10, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 10, 2022

Current assignee @tgolen is eligible for the External assigner, not assigning anyone new.

@melvin-bot melvin-bot bot changed the title BUG - Create room - Workspace is not selected by default if one workspace only available in the account [$250] BUG - Create room - Workspace is not selected by default if one workspace only available in the account Oct 10, 2022
@aimane-chnaif
Copy link
Contributor

Proposal

Solution 1: The case when this should be applied to Create room page only
https://github.com/Expensify/App/blob/main/src/pages/workspace/WorkspaceNewRoomPage.js

    componentDidMount() {
        // Workspaces are policies with type === 'free'
        const workspaces = _.filter(this.props.policies, policy => policy && policy.type === CONST.POLICY.TYPE.FREE);
-       this.setState({workspaceOptions: _.map(workspaces, policy => ({label: policy.name, key: policy.id, value: policy.id}))});
+       this.setState({
+           policyID: workspaces.length === 1 ? workspaces[0].id : '',
+           workspaceOptions: _.map(workspaces, policy => ({label: policy.name, key: policy.id, value: policy.id})),
+       });
    }

Solution 2: The case when this should be applied to all pickers throughout the app
https://github.com/Expensify/App/blob/main/src/components/Picker/BasePicker/index.js

    constructor(props) {
        super(props);

        this.executeOnCloseAndOnBlur = this.executeOnCloseAndOnBlur.bind(this);
+       this.setDefaultValue = this.setDefaultValue.bind(this);
    }
+
+   componentDidMount() {
+       this.setDefaultValue();
+   }

+   componentDidUpdate(prevProps) {
+       if (prevProps.items === this.props.items) {
+           return;
+       }
+       this.setDefaultValue();
+   }
+
+   setDefaultValue() {
+       if (this.props.value || !this.props.items || this.props.items.length !== 1 || !this.props.onInputChange) {
+           return;
+       }
+       this.props.onInputChange(this.props.items[0].key);
+   }

Demo:

Web:

demo2.mov

iOS:

demo1.mov

@sobitneupane
Copy link
Contributor

@rushatgabhane Your proposal will change value in the dropdown or the picker but the changed value is not passed to the parent component.

@aimane-chnaif Soluion 2 from above proposal looks good to me. His proposal is to apply the change to a general component Picker. @aimane-chnaif Please do test all the components using Picker to make sure it doesn't cause any regression and output is as expected while creating PR.

cc: @tgolen

🎀👀🎀 C+ reviewed

@tgolen
Copy link
Contributor

tgolen commented Oct 11, 2022

Yeah, I like the proposal for applying it to all pickers 👍 🟢

@aimane-chnaif
Copy link
Contributor

@tgolen may I go ahead raising PR? I see 🟢 but still asking because I am not assigned yet.

@melvin-bot melvin-bot bot added the Overdue label Oct 13, 2022
@tgolen
Copy link
Contributor

tgolen commented Oct 13, 2022

I think @isabelastisser needs to create an upwork job and assign it to you next. But, I think you're OK to start on a PR and we'll get they payments and job figured out in the meantime.

@melvin-bot melvin-bot bot removed the Overdue label Oct 13, 2022
@aimane-chnaif
Copy link
Contributor

I think @isabelastisser needs to create an upwork job and assign it to you next. But, I think you're OK to start on a PR and we'll get they payments and job figured out in the meantime.

Thanks @tgolen. upwork assign is no problem. that can be later. but I just asked about GH assign. btw will raise PR soon

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Help Wanted Apply this label when an issue is open to proposals by contributors Daily KSv2 labels Oct 13, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 13, 2022

📣 @aimane-chnaif You have been assigned to this job by @tgolen!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@tgolen
Copy link
Contributor

tgolen commented Oct 13, 2022

Cool, I'll do the GH assignment for you.

@isabelastisser
Copy link
Contributor

Hey @tgolen , I'm still getting familiar with this process, so do I still need to create an Upwork job and assign it to @aimane-chnaif ? It looks like this has been done here, but I wanted to confirm. Thanks!

@puneetlath puneetlath added the Bug Something is broken. Auto assigns a BugZero manager. label Oct 19, 2022
@tgolen
Copy link
Contributor

tgolen commented Oct 23, 2022

I honestly don't really know. Can you check SO to see what the current process is supposed to be? It's constantly evolving.

@alexpensify
Copy link
Contributor

alexpensify commented Oct 23, 2022

@isabelastisser - I was confused too. On Friday, I asked here and confirmed that I did need to create a job for the GH that I was tagged as external. I think that's the next step here too.

@isabelastisser
Copy link
Contributor

Hi @aimane-chnaif, can you please let me know if you were already assigned this job in Upwork? If not, I will create one today. Thanks!

@aimane-chnaif
Copy link
Contributor

Hi @aimane-chnaif, can you please let me know if you were already assigned this job in Upwork? If not, I will create one today. Thanks!

@isabelastisser I am not assigned yet

@isabelastisser
Copy link
Contributor

@isabelastisser
Copy link
Contributor

@kavimuru , I couldn't find you on Upwork, can you please apply for the job there so that I can hire you? Thanks!

@yanjankaf
Copy link

don't you guys have something to close these issues? its already solved, why is this still open?

@aimane-chnaif
Copy link
Contributor

@tgolen PR has not been merged yet. I think you just forgot to click Merge button after Approve

@melvin-bot
Copy link

melvin-bot bot commented Nov 2, 2022

BugZero Checklist: The PR fixing this issue has been merged! The following checklist will need to be completed before the issue can be closed:

  • A regression test has been added or updated so that the same bug will not reach production again. Link to the updated test here:
  • The PR that introduced the bug has been identified. Link to the PR:
  • The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • A discussion in #contributor-plus has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • Payment has been made to the issue reporter (if applicable)
  • Payment has been made to the contributor that fixed the issue (if applicable)
  • Payment has been made to the contributor+ that helped on the issue (if applicable)

@tgolen
Copy link
Contributor

tgolen commented Nov 2, 2022 via email

@melvin-bot melvin-bot bot added the Overdue label Nov 11, 2022
@sobitneupane
Copy link
Contributor

Waiting payment. PR was deployed to production 9 days ago.
Screenshot 2022-11-14 at 09 51 46
cc: @isabelastisser

@melvin-bot melvin-bot bot removed the Overdue label Nov 14, 2022
@isabelastisser
Copy link
Contributor

The Contributor and Contributor+ have been paid, the contract has been completed, and the Upwork post has been closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
None yet
Development

No branches or pull requests

9 participants