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

[HOLD for payment 2024-05-02] [Mentions v2] Update auto-complete widget to perform online search when needed #39534

Closed
rlinoz opened this issue Apr 3, 2024 · 39 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Design Engineering NewFeature Something to build that is a new item.

Comments

@rlinoz
Copy link
Contributor

rlinoz commented Apr 3, 2024

HOLD #39532
HOLD #39505

When mentioning a room and the auto-complete widget has less than 5 results we will perform a request to search for reports that we might not have locally, this request will call the command SearchForRoomsToMention which will take a "query" and a policyID as parameters. The result of the call will be merge into onyx with the other reports and will automatically update the auto-complete widget with the new results.

While performing the search the auto-complete widget must show a loading indicator, as its shown here

Details can be found here in the Mentions V2 design doc.

@melvin-bot melvin-bot bot added the Monthly KSv2 label Apr 8, 2024
@war-in
Copy link
Contributor

war-in commented Apr 12, 2024

Hi, I'm Marcin from SoftwareMansion and I would like to work on this one :) could you please assign me? 🙏

@rlinoz
Copy link
Contributor Author

rlinoz commented Apr 12, 2024

@war-in keep in mind that we still need this PR https://github.com/Expensify/Auth/pull/10483 to get merged to unblock this.

@rlinoz rlinoz added Daily KSv2 and removed Monthly KSv2 labels Apr 12, 2024
@war-in
Copy link
Contributor

war-in commented Apr 12, 2024

@rlinoz Yes I know :) I'm just starting to investigate it and would like to be assigned to avoid duplication

@rlinoz rlinoz added the NewFeature Something to build that is a new item. label Apr 12, 2024
Copy link

melvin-bot bot commented Apr 12, 2024

Triggered auto assignment to @jliexpensify (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Apr 12, 2024
Copy link

melvin-bot bot commented Apr 12, 2024

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

@robertKozik
Copy link
Contributor

@war-in keep in mind that we still need this PR https://github.com/Expensify/Auth/pull/10483 to get merged to unblock this.

Wondering what's the ETA for this?

@rlinoz
Copy link
Contributor Author

rlinoz commented Apr 12, 2024

It is in review, I think it will get merged today, but deployed only on monday.

Although, the SearchForRoomsToMention command is live, it just doesn't return the chatType and policyID with the report, so you can add the api call and ensure that it is being called correctly.

@rlinoz
Copy link
Contributor Author

rlinoz commented Apr 12, 2024

https://github.com/Expensify/Auth/pull/10483 is merged, waiting for deploy now.

@rlinoz rlinoz self-assigned this Apr 12, 2024
@jliexpensify
Copy link
Contributor

No payment needed right?

@rlinoz
Copy link
Contributor Author

rlinoz commented Apr 15, 2024

@jliexpensify I think we will have a C+ reviewing the PR later, so we will need payment at some point

@war-in
Copy link
Contributor

war-in commented Apr 16, 2024

@rlinoz @puneetlath We just noticed that there is a bug when trying to click on a single mention suggestion on iOS. After some investigation, it turned out that the issue is connected to FlashList usage in the SuggestionMention. It is somehow affected by the FlatList which is used to display messages. The workaround would be to replace FlashList with FlatList (it works correctly, but haven't tested it too much).

Bug exists on main branch too so it is not connected to changes from my branch

Example:

Screen.Recording.2024-04-16.at.10.54.46.mov

@rlinoz
Copy link
Contributor Author

rlinoz commented Apr 16, 2024

Last backend PR is deployed to prod.

@rlinoz
Copy link
Contributor Author

rlinoz commented Apr 16, 2024

@war-in is that happening only on iOS?

@rlinoz rlinoz changed the title [HOLD Mentions v2] Update auto-complete widget to perform online search when needed [Mentions v2] Update auto-complete widget to perform online search when needed Apr 16, 2024
@rlinoz
Copy link
Contributor Author

rlinoz commented Apr 16, 2024

Anyway, it looks like there is a lot going on with FlashList right now, I think it would be better to fix this bug in another issue if it still exists after all the PR's in that issue are merged.

cc: @puneetlath @war-in

@war-in
Copy link
Contributor

war-in commented Apr 17, 2024

is that happening only on iOS?

Yeah, checked on other devices and they are fine.

@puneetlath
Copy link
Contributor

Sorry for being dense, I couldn't tell from the video what the bug was. Mind explaining?

@rlinoz
Copy link
Contributor Author

rlinoz commented Apr 17, 2024

If the auto complete has only one item it is not possible to select it, if I understood correctly

@puneetlath
Copy link
Contributor

Ohhh I see. Hm, yeah, that's not ideal. But I also agree about handling it separately if that's an existing bug.

Copy link

melvin-bot bot commented Apr 22, 2024

Triggered auto assignment to @shawnborton (Design), see these Stack Overflow questions for more details.

@shawnborton
Copy link
Contributor

Let me know if anything is needed from Design here.

@rlinoz
Copy link
Contributor Author

rlinoz commented Apr 22, 2024

Oh I tagged you on the PR #40232 (comment) sorry

@shawnborton
Copy link
Contributor

Sounds good, see ya over there!

@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 25, 2024
@melvin-bot melvin-bot bot changed the title [Mentions v2] Update auto-complete widget to perform online search when needed [HOLD for payment 2024-05-02] [Mentions v2] Update auto-complete widget to perform online search when needed Apr 25, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Apr 25, 2024
Copy link

melvin-bot bot commented Apr 25, 2024

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

Copy link

melvin-bot bot commented Apr 25, 2024

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

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

  • @war-in does not require payment (Contractor)

Copy link

melvin-bot bot commented Apr 25, 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:

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

@rlinoz
Copy link
Contributor Author

rlinoz commented Apr 25, 2024

@parasharrajat reviewed the PR.

@jliexpensify
Copy link
Contributor

Ok cool, so this should be the Payment Summary right?

@parasharrajat - anything needed for the checklist?

@war-in
Copy link
Contributor

war-in commented Apr 26, 2024

@jliexpensify should I create a regression test? There is a comment from malvin-bot but I haven't seen this before so I need to ask 😅

@jliexpensify
Copy link
Contributor

Hi @war-in - sure, if you think one is needed, feel free to propose the steps and I can send this over to QA.

@puneetlath
Copy link
Contributor

@rlinoz we should actually probably put together a set of regression tests for room mentions in general.

@rlinoz
Copy link
Contributor Author

rlinoz commented Apr 26, 2024

Oh interesting, I agree. I will create some steps and share them in slack to get comments.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels May 1, 2024
@jliexpensify
Copy link
Contributor

@jliexpensify
Copy link
Contributor

@rlinoz there's nothing for me to do in terms of payments, but let me know if you come up with a Regression Test and I can create the GH. Otherwise, feel free to close this GH!

@rlinoz
Copy link
Contributor Author

rlinoz commented May 2, 2024

Started a slack thread here for regression tests of the entire feature, I will try to open the GH to add the tests by tomorrow.

We can close this one.

@rlinoz rlinoz closed this as completed May 2, 2024
@parasharrajat
Copy link
Member

As per this thread https://expensify.slack.com/archives/C02NK2DQWUX/p1715272276516799?thread_ts=1715190852.677769&cid=C02NK2DQWUX, the price should be 500.

Could you please update the summary @jliexpensify ?

@jliexpensify
Copy link
Contributor

NEW PAYMENT SUMMARY

@parasharrajat
Copy link
Member

Payment requested as per #39534 (comment)

@JmillsExpensify
Copy link

$500 approved for @parasharrajat

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 Daily KSv2 Design Engineering NewFeature Something to build that is a new item.
Projects
None yet
Development

No branches or pull requests

8 participants