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

[PAID] [$1000] Feature Request: Implement an option to go to #admin room from the workspace editor #18896

Closed
kavimuru opened this issue May 13, 2023 · 53 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.

Comments

@kavimuru
Copy link

kavimuru commented May 13, 2023

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


Problem:

There is no option to go to admins room for the workspace from the workspace editor

Solution:

Go to # admin room and Go to #announce room can be added in the 3 dot overflow

Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:
2

Expensify/Expensify Issue URL:
Issue reported by: @quinthar
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1683935683755399

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01d9a45172b145ab15
  • Upwork Job ID: 1658049048662507520
  • Last Price Increase: 2023-05-15
@kavimuru kavimuru added Weekly KSv2 NewFeature Something to build that is a new item. labels May 13, 2023
@melvin-bot
Copy link

melvin-bot bot commented May 13, 2023

@alitoshmatov
Copy link
Contributor

alitoshmatov commented May 13, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

Feature Request: Implement an option to go to #admin room from the workspace editor

What is the root cause of that problem?

N/A

What changes do you think we should make in order to solve the problem?

We should add 2 new menu items here, 1. Go to #admin room 2. Go to #anounce room

icon: Expensicons.Trashcan,
text: props.translate('workspace.common.delete'),
onSelected: () => setIsDeleteModalOpen(true),
},
]}
threeDotsAnchorPosition={styles.threeDotsPopoverOffset}
/>

When clicked, we should do something like this:

const goTo = (type: CONST.REPORT.CHAT_TYPE.POLICY_ADMINS | CONST.REPORT.CHAT_TYPE.POLICY_ANNOUNCE)=>{
        const report = _.find(props.reports, (report) => report && report.policyID === policy.id && report.chatType === type);
        Navigation.navigate(ROUTES.getReportRoute(report.reportID));
    }

which is finding a report that belongs to current policy and is a #admin or #announce room

What alternative solutions did you explore? (Optional)

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels May 13, 2023
@melvin-bot
Copy link

melvin-bot bot commented May 13, 2023

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

@dylanexpensify dylanexpensify added the External Added to denote the issue can be worked on by a contributor label May 15, 2023
@melvin-bot melvin-bot bot changed the title Feature Request: Implement an option to go to #admin room from the workspace editor [$1000] Feature Request: Implement an option to go to #admin room from the workspace editor May 15, 2023
@melvin-bot
Copy link

melvin-bot bot commented May 15, 2023

Job added to Upwork: https://www.upwork.com/jobs/~01d9a45172b145ab15

@melvin-bot
Copy link

melvin-bot bot commented May 15, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented May 15, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label May 15, 2023
@melvin-bot
Copy link

melvin-bot bot commented May 15, 2023

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

@abdulrahuman5196
Copy link
Contributor

abdulrahuman5196 commented May 15, 2023

@dylanexpensify Before approving proposals, I wanted to confirm. Are we good with adding new menu items to the workspace menus from UX perspective or Are we planning for different/better UX? Since it seemed to be a suggestion from slack thread.

Note: And If are good with new menu items, we would need icons and text, so it would be good to tag design team member as well here whom we can reach in case of clarifications.

@abdulrahuman5196
Copy link
Contributor

@dylanexpensify / @marcaaron Could you kindly confirm on #18896 (comment) ? So that we can keep the issue moving?

@marcaaron
Copy link
Contributor

Are we good with adding new menu items to the workspace menus from UX perspective

That's what the issue is asking for so I'd say we are good with it.

Are we planning for different/better UX? Since it seemed to be a suggestion from slack thread.

Not totally sure - can you summarize what the Slack thread says and tag in whoever is involved on that to get feedback?

we would need icons and text, so it would be good to tag design team member as well here whom we can reach in case of clarifications.

These rooms already have default icons so I'd say we should just re-use those, but yes, curious for @shawnborton's take.

@abdulrahuman5196
Copy link
Contributor

abdulrahuman5196 commented May 17, 2023

Thank you marc@ for the confirmation. I just wanted to confirm if we looking out for different UX just in case.

@abdulrahuman5196
Copy link
Contributor

Since this issue is a straight implementation, I think this proposal - #18896 (comment) by @alitoshmatov is good

🎀 👀 🎀 C+ reviewed
@marcaaron

@shawnborton
Copy link
Contributor

These rooms already have default icons so I'd say we should just re-use those, but yes, curious for @shawnborton's take.

Actually I think we want to remove the room-specific icons and always use the workspace avatar. cc @MitchExpensify @grgia @JmillsExpensify not sure if this issue was ever created but it was decided during the threads predesigns, but we should implement that as soon as we can.

As for the overflow menu, we can just reuse the hashtag room icon we use in the "New room" option in the FAB for this. And then the label would say "Go to #admins room" or something

@Prince-Mendiratta
Copy link
Contributor

@shawnborton Should we also add an option to go to #announce room? If so, what should be the icon for it?

@shawnborton
Copy link
Contributor

shawnborton commented May 18, 2023 via email

@marcaaron
Copy link
Contributor

LGTM @alitoshmatov just remember that not every workspace member can see the #admins room so that will need to be accounted for.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label May 18, 2023
@melvin-bot
Copy link

melvin-bot bot commented May 18, 2023

📣 @alitoshmatov You have been assigned to this job by @marcaaron!
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 📖

@alitoshmatov
Copy link
Contributor

@marcaaron I thought anyone who has access to workspace page is an admin, thus can see #admin, am I wrong?

@abdulrahuman5196
Copy link
Contributor

Oh. Sorry. I thought internal engineers raise it in an internal chat room. Atleast that's how it went for my other PRs 😵‍💫

I have raised the translation confirmation in slack again - https://expensify.slack.com/archives/C01GTK53T8Q/p1684826176793429

@alitoshmatov Kindly do check up on that as well.

@alitoshmatov
Copy link
Contributor

@strepanier03 Can you help us here to move our PR forward, since @marcaaron is OOO

@abdulrahuman5196
Copy link
Contributor

abdulrahuman5196 commented May 24, 2023

@strepanier03 C+ approval and reviewer checklist is complete. kindly involve a new engineer.

PR - #19295

@melvin-bot
Copy link

melvin-bot bot commented May 24, 2023

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

@strepanier03
Copy link
Contributor

@hayata-suenaga - The previous engineer @marcaaron is out and we needed a new one, congrats on luck of the draw.

Latest update for you:

C+ approval and reviewer checklist is complete. kindly involve a new engineer.

PR - #19295

@abdulrahuman5196
Copy link
Contributor

@hayata-suenaga gentle ping on the review.

@hayata-suenaga
Copy link
Contributor

ah, I missed this

@strepanier03 when you reassign an engineer, can you assign them as a reviewer to the PR? that's way, the review is displayed on my K2 dashboard's review section.

Screenshot 2023-05-26 at 3 58 20 PM

@hayata-suenaga
Copy link
Contributor

hayata-suenaga commented May 26, 2023

merged

@strepanier03
Copy link
Contributor

@hayata-suenaga - absolutely, thank you so much for the request, I didn't even think of that but will make sure I do from now on.

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels May 31, 2023
@melvin-bot melvin-bot bot changed the title [$1000] Feature Request: Implement an option to go to #admin room from the workspace editor [HOLD for payment 2023-06-07] [$1000] Feature Request: Implement an option to go to #admin room from the workspace editor May 31, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label May 31, 2023
@melvin-bot
Copy link

melvin-bot bot commented May 31, 2023

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot
Copy link

melvin-bot bot commented May 31, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.20-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-06-07. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

  • External issue reporter - Internal report - no reporting bonus
  • Contributor that fixed the issue - @alitoshmatov - $1000
  • Contributor+ that helped on the issue and/or PR - @abdulrahuman5196 - $1000

Speed bonus assessment: Contributor hired on May 18 / PR merged on May 26 = 6 business days = Not eligible for speed bonus or penalty.

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

@melvin-bot
Copy link

melvin-bot bot commented May 31, 2023

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

  • [@abdulrahuman5196] The PR that introduced the bug has been identified. Link to the PR: N/A - New feature
  • [@abdulrahuman5196] 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: N/A - New feature
  • [@abdulrahuman5196] A discussion in #expensify-bugs 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: N/A - New feature
  • [@abdulrahuman5196] Determine if we should create a regression test for this bug. - Yes
  • [@abdulrahuman5196] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again. - Done
  • [@strepanier03 / @dylanexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: https://github.com/Expensify/Expensify/issues/290035

@abdulrahuman5196
Copy link
Contributor

BugZero Checklist:

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 #expensify-bugs 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:

New feature. Not a regression

Determine if we should create a regression test for this bug.

Yes.

If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

  1. Go to Settings -> Workspaces -> Workspace page
  2. Press three dots
  3. Make sure that followings are present a)Go to #admins room b)Go to #announce room
  4. Press Go to #admins room
  5. It should navigate to the #admins room of the workspace
  6. Repeat steps 1 to 3
  7. Press Go to #announce room
  8. It should navigate to the #announce room of the workspace

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jun 6, 2023
@alitoshmatov
Copy link
Contributor

Looks like this one is ready for payment

@strepanier03
Copy link
Contributor

@alitoshmatov and @abdulrahuman5196 - I've hired you both the Upwork job. I'll check in again this evening to see if I can pay either out.

@strepanier03
Copy link
Contributor

Added reg test request.

@strepanier03
Copy link
Contributor

Looks like contracts are still waiting to be accepted so I'll check in in the morning to try to pay again.

@alitoshmatov
Copy link
Contributor

@strepanier03 accepted contract in upwork

@strepanier03 strepanier03 changed the title [HOLD for payment 2023-06-07] [$1000] Feature Request: Implement an option to go to #admin room from the workspace editor [PAID] [$1000] Feature Request: Implement an option to go to #admin room from the workspace editor Jun 8, 2023
@strepanier03
Copy link
Contributor

I've paid everyone, ended contracts, and left feedback.

Thank you everyone for the hard work fixing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.
Projects
None yet
Development

No branches or pull requests

9 participants