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

[Awaiting C+ Steps] [Guided Setup Stage 2] Add support for read-only messages (Phase 2) #38773

Closed
francoisl opened this issue Mar 21, 2024 · 36 comments
Assignees
Labels
Internal Requires API changes or must be handled by Expensify staff NewFeature Something to build that is a new item. Weekly KSv2

Comments

@francoisl
Copy link
Contributor

francoisl commented Mar 21, 2024

Part of the "wave-collect – Build Stage 2 of Guided Setup" project

Main issue: https://github.com/Expensify/Expensify/issues/356685
Doc section: https://docs.google.com/document/d/10bhCv6XtzzqEZQ9tzxFwAz4lowgjOhsFqz13BMq6Iu4/edit#heading=h.9c9l6vfd8y0n
Project: https://github.com/orgs/Expensify/projects/129

Feature Description

Use the new permissions = ["read"] prop on report objects to hide the composer. When the system account ("Expensify persona") is the only other participant of the report, display a static message instead of the composer instead

cc @barttom

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01b804c59943f9fb25
  • Upwork Job ID: 1770905188139683840
  • Last Price Increase: 2024-03-21
Issue OwnerCurrent Issue Owner: @hungvu193
@francoisl francoisl added Internal Requires API changes or must be handled by Expensify staff NewFeature Something to build that is a new item. labels Mar 21, 2024
Copy link

melvin-bot bot commented Mar 21, 2024

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

Copy link

melvin-bot bot commented Mar 21, 2024

Triggered auto assignment to Contributor Plus for review of internal employee PR - @rojiphil (Internal)

Copy link

melvin-bot bot commented Mar 21, 2024

@melvin-bot melvin-bot bot added the Weekly KSv2 label Mar 21, 2024
@barttom
Copy link
Contributor

barttom commented Mar 22, 2024

Hello! I'm Bartek from Callstack, an expert contributor group, and I would like to work on this issue.

@rojiphil
Copy link
Contributor

@twisterdotcom Can you please grant me access to this document mentioned in OP? I have requested now. My email is rojiphil@gmail.com

@twisterdotcom
Copy link
Contributor

Ah I see we can share design docs for C+: https://stackoverflowteams.com/c/expensify/questions/11623/11624#11624.

@twisterdotcom
Copy link
Contributor

Just asking in Slack whether it's best to share with individuals or with all C+ via contributor_plus@expensify.com: https://expensify.slack.com/archives/C01SKUP7QR0/p1711544698772059

@twisterdotcom
Copy link
Contributor

Okay shared only as a commenter rojiphil@gmail.com / @rojiphil.

@melvin-bot melvin-bot bot added the Overdue label Apr 5, 2024
@twisterdotcom
Copy link
Contributor

How are we doing here @barttom?

@melvin-bot melvin-bot bot removed the Overdue label Apr 8, 2024
@barttom
Copy link
Contributor

barttom commented Apr 8, 2024

@twisterdotcom Basically, it's finished, but we are not able to test it without manual code changes and currently, I'm working on #38771

We can test it when #38769 has been finished.
Then I can implement this in. the app and I'll send this one to review with #38774 - that will allow us to test this issue.

@twisterdotcom twisterdotcom changed the title [Guided Setup Stage 2] Add support for read-only messages (Phase 2) [HOLD on #38769] [Guided Setup Stage 2] Add support for read-only messages (Phase 2) Apr 8, 2024
@twisterdotcom
Copy link
Contributor

Nice! Thanks

@rezkiy37
Copy link
Contributor

Hi, I’m Michael (Mykhailo) from Callstack and I would like to work on this issue.

@twisterdotcom
Copy link
Contributor

How is this PR going? We just held on #38769 right?

@melvin-bot melvin-bot bot removed the Overdue label Apr 19, 2024
@rezkiy37
Copy link
Contributor

How is this PR going? We just held on #38769 right?

I prepared the draft PR. Once #38769 is merged, I can start to test immediately.

@rezkiy37
Copy link
Contributor

Guys, should we block those actions for read-only chats?

Screenshot 2024-04-22 at 12 52 08

cc @francoisl @twisterdotcom @mountiny

@mountiny
Copy link
Contributor

Yeah I feel like we should not allow creating threads there

@rezkiy37
Copy link
Contributor

The issue is not on hold anymore. But the PR for this issue is on hold and waiting for #40678.

@rezkiy37
Copy link
Contributor

Hey!
I just wanted to let you know about my vacation for next week. I will be back on 06.05.2024. Meanwhile, I asked @waterim to take over this issue.
See you soon!

Copy link

melvin-bot bot commented Apr 29, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Apr 29, 2024
@melvin-bot melvin-bot bot changed the title [Guided Setup Stage 2] Add support for read-only messages (Phase 2) [HOLD for payment 2024-05-06] [Guided Setup Stage 2] Add support for read-only messages (Phase 2) Apr 29, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Apr 29, 2024
Copy link

melvin-bot bot commented Apr 29, 2024

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

Copy link

melvin-bot bot commented Apr 29, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.67-7 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 2024-05-06. 🎊

For reference, here are some details about the assignees on this issue:

  • @rojiphil requires payment (Needs manual offer from BZ)
  • @barttom does not require payment (Contractor)
  • @rezkiy37 does not require payment (Contractor)

Copy link

melvin-bot bot commented Apr 29, 2024

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

  • [@rojiphil] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
  • [@twisterdotcom] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

@hungvu193
Copy link
Contributor

@mountiny Since I reviewed the PR, can you assign me to this issue please? Thank you 😄

@twisterdotcom
Copy link
Contributor

Hey, sorry just getting to this - apologies I was out last week. I only need to pay @rojiphil and @hungvu193 via Upwork, for $250 each right? Or did @rojiphil not review an associated PR, but @hungvu193 did?

@melvin-bot melvin-bot bot removed the Overdue label May 14, 2024
@hungvu193
Copy link
Contributor

Hey, sorry just getting to this - apologies I was out last week. I only need to pay @rojiphil and @hungvu193 via Upwork, for $250 each right? Or did @rojiphil not review an associated PR, but @hungvu193 did?

I think it's just me because I reviewed the PR.

@rojiphil
Copy link
Contributor

Hey, sorry just getting to this - apologies I was out last week. I only need to pay @rojiphil and @hungvu193 via Upwork, for $250 each right? Or did @rojiphil not review an associated PR, but @hungvu193 did?

@twisterdotcom That's correct. No payment for me as I did not review the PR.

@twisterdotcom
Copy link
Contributor

twisterdotcom commented May 20, 2024

Payment Summary:

@twisterdotcom
Copy link
Contributor

@hungvu193 - want to propose the regression steps here?

@twisterdotcom twisterdotcom removed the Awaiting Payment Auto-added when associated PR is deployed to production label May 20, 2024
@twisterdotcom twisterdotcom changed the title [HOLD for payment 2024-05-06] [Guided Setup Stage 2] Add support for read-only messages (Phase 2) [Awaiting C+ Steps] [Guided Setup Stage 2] Add support for read-only messages (Phase 2) May 20, 2024
@hungvu193
Copy link
Contributor

@hungvu193 - want to propose the regression steps here?

This is new feature so I don't think we need regression test here

@twisterdotcom
Copy link
Contributor

Why would we not create regression steps for a new feature though? It is part of the normal new feature checklist: #38773 (comment)

@hungvu193
Copy link
Contributor

Why would we not create regression steps for a new feature though? It is part of the normal new feature checklist: #38773 (comment)

Sorry I missed it 🥲

@hungvu193
Copy link
Contributor

Regression test:

  1. Complete the onboarding flow.
  2. Open the chat.
  3. Verify that the composer is hidden.
  4. Verify that a new footer component exists.
  5. Open a task report.
  6. Verify that the composer is hidden.
  7. Verify that a new footer component exists.
  8. Verify that you cannot complete or delete a task.

Do we 👍 or 👎 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internal Requires API changes or must be handled by Expensify staff NewFeature Something to build that is a new item. Weekly KSv2
Projects
No open projects
Archived in project
Development

No branches or pull requests

8 participants