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 2023-04-03] Unwanted double quotes in notification after ... for bigger messages #14849

Closed
1 task
kavimuru opened this issue Feb 6, 2023 · 83 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 Internal Requires API changes or must be handled by Expensify staff

Comments

@kavimuru
Copy link

kavimuru commented Feb 6, 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!


Action Performed:

  1. Open the app and login with user A
  2. Open the app in android app and login with user B
  3. Send long message from user A to user B

Expected Result:

Larger notifcations should end with ... in the end

Actual Result:

Larger notification ends with ..." in the end (App is adding unwanted double quotes)

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android / native

Version Number: 1.2.65-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

double.quotes.in.notification.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1675500083078039

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01441300c2ff47e240
  • Upwork Job ID: 1624093978140704768
  • Last Price Increase: 2023-02-17
@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 6, 2023
@melvin-bot
Copy link

melvin-bot bot commented Feb 6, 2023

Triggered auto assignment to @zanyrenney (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot melvin-bot bot locked and limited conversation to collaborators Feb 6, 2023
@melvin-bot melvin-bot bot added the Overdue label Feb 8, 2023
@zanyrenney
Copy link
Contributor

Need to try and reproduce! Haven't been able to so far

@melvin-bot melvin-bot bot removed the Overdue label Feb 9, 2023
@zanyrenney zanyrenney added the External Added to denote the issue can be worked on by a contributor label Feb 10, 2023
@melvin-bot melvin-bot bot unlocked this conversation Feb 10, 2023
@melvin-bot melvin-bot bot changed the title Unwanted double quotes in notification after ... for bigger messages [$1000] Unwanted double quotes in notification after ... for bigger messages Feb 10, 2023
@MelvinBot
Copy link

Job added to Upwork: https://www.upwork.com/jobs/~01441300c2ff47e240

@MelvinBot
Copy link

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

@MelvinBot
Copy link

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

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

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

@alexxxwork
Copy link
Contributor

I've posted a Proposal for similar problem.
To say short I suppose this logic is on the airship service side
#14715 (comment)

@parasharrajat
Copy link
Member

@alexxxwork That can not be counted as a proposal as it does not propose any solutions with the analysis.

@alexxxwork
Copy link
Contributor

alexxxwork commented Feb 11, 2023

@alexxxwork That can not be counted as a proposal as it does not propose any solutions with the analysis.

@parasharrajat Yeah, of course. But I think the most right thing to do is to address the problem to airship service. It would be a hard task to write rules for notification messages truncation on app side without thorough knowledge of how this work inside the airship service.

@melvin-bot melvin-bot bot added the Overdue label Feb 13, 2023
@parasharrajat
Copy link
Member

parasharrajat commented Feb 13, 2023

Ok, I tried to find the source code related to Notifications but no luck. I am not an Android guy so don't know where to look exactly. Any help will be appreciated.

Yeah, of course. But I think the most right thing to do is to address the problem to airship service

Ok, @alexxxwork. IMO, Maybe this can easily be resolved from the app side with the proper configuration of the notification builder. I don't think we need to know anything about airship service. the Logic should already be there in the source.

What do you propose? How can you help to advance this issue?

@melvin-bot melvin-bot bot removed the Overdue label Feb 13, 2023
@parasharrajat
Copy link
Member

Waiting for proposals.

@alexxxwork
Copy link
Contributor

alexxxwork commented Feb 13, 2023

Ok, @alexxxwork. IMO, Maybe this can easily be resolved from the app side with the proper configuration of the notification builder. I don't think we need to know anything about airship service. the Logic should already be there in the source.

@parasharrajat ok, we could go this way - we should count symbols on our side and truncate string sent to notification service. I'll write down a proposal by template.

@zanyrenney
Copy link
Contributor

thanks for communicating @parasharrajat - still waiting for proposals here!

@alexxxwork
Copy link
Contributor

alexxxwork commented Feb 15, 2023

Proposal

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

We are trying to resolve an issue in long string coming to notifications on android native. Strings more than 115 ascii symbols get truncated with ...'' in the end.

What is the root cause of that problem?

The root cause of the problem is limits of the airship service for android plaform doc

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

We should truncate string on our side here

What alternative solutions did you explore? (Optional)

We could address this issue upstream to airship

@parasharrajat
Copy link
Member

Now sure, I understand the proposal.

  • How can you trim the message?
  • How will it solve the problem? What is the run-through?

Don't we have any API on the builder or something to customize the notification format? I like your optional solution. Do you want to raise it with them?

@dhanashree-sawant
Copy link

Thanks @zanyrenney , I have accepted the invite.

@alexxxwork
Copy link
Contributor

alexxxwork commented Apr 5, 2023

Ok. Thanks, both. @alexxxwork It seems we are taking these issues internally as it's been almost 3 weeks and we have not heard from Airship. We also agreed to compensate you for the same #14715 (comment) on another issue.

Thanks for your help.

@parasharrajat @zanyrenney Hey, Rajat, Zany. Am I eligible for compensation here?

@Julesssss
Copy link
Contributor

Hi @alexxxwork. As we didn't assign you to this issue, I don't think so. However, you seem to be eligible for payment on this linked issue. Which is yet to be completely resolved.

@parasharrajat
Copy link
Member

Note: @alexxxwork is not eligible for full payment of #14715 (comment). The issue was solved internally and we agreed to pay for the research done before this issue was taken internally.

@dhanashree-sawant
Copy link

Hi @zanyrenney, thanks for the offer on upwork but when I tried to accept it now, it says that job is no longer available, can you have a look once you are available?

@Puneet-here
Copy link
Contributor

Hi @zanyrenney, this bug was originally reported by me here

@MelvinBot
Copy link

📣 @Puneet-here! 📣

Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.

Screen Shot 2022-11-16 at 4 42 54 PM

Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@parasharrajat
Copy link
Member

@Puneet-here, Yeah It seems so. But there was a question raised to that thread but it was not pursued thus I think QA didn't export it. IMO, That report is valid but outdated.

But not my call...

@melvin-bot melvin-bot bot added the Overdue label Apr 10, 2023
@MelvinBot
Copy link

@Julesssss, @parasharrajat, @zanyrenney Whoops! This issue is 2 days overdue. Let's get this updated quick!

1 similar comment
@MelvinBot
Copy link

@Julesssss, @parasharrajat, @zanyrenney Whoops! This issue is 2 days overdue. Let's get this updated quick!

@zanyrenney
Copy link
Contributor

zanyrenney commented Apr 11, 2023

Hmm, sorry I am confused now. @dhanashree-sawant](https://github.com/dhanashree-sawant)
dhanashree-sawant i thought you were the reporter. If Puneet-here is the reporter, then you wouldn't get the payment. Can you explain what you're due payment for in this case @dhanashree-sawant ?

I'd like to ensure we get the correct reporter paid out but there seems to be some confusion here as to who that is.

@melvin-bot melvin-bot bot removed the Overdue label Apr 11, 2023
@dhanashree-sawant
Copy link

Hi @zanyrenney, I agree, Puneet had raised the concern but it was 5 months back from this report and we never went ahead with it until I saw the issue again and reported it (I was not aware of the report as it was in expensify-open-source platform until Puneet pointed it out). Its on you on how to handle the payments.

@Julesssss
Copy link
Contributor

Ah right. Seems like a fair mistake.

@Julesssss
Copy link
Contributor

This is a tough call. Given that no further action was taken on the original issue I think the newer bug report should receive the bounty, personally. It looks like it was simply missed originally :/

@zanyrenney
Copy link
Contributor

Thanks @Julesssss I agree.

@parasharrajat
Copy link
Member

This should be ready to be closed.

@kbecciv
Copy link

kbecciv commented Sep 13, 2023

Reopeing the issue
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1694364092518829

@kbecciv kbecciv reopened this Sep 13, 2023
@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Sep 13, 2023
@parasharrajat
Copy link
Member

@kbecciv Please reset the title. Thanks

@parasharrajat
Copy link
Member

@Julesssss This will also be a backend change.

@Julesssss
Copy link
Contributor

This is

Reopeing the issue Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1694364092518829

Can you please open a new issue and assign me? This one is 6 months old and while similar, it will require a different solution

@kbecciv
Copy link

kbecciv commented Sep 18, 2023

Sure, will do that @Julesssss

@kbecciv
Copy link

kbecciv commented Sep 18, 2023

Link new issue #27663

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 Internal Requires API changes or must be handled by Expensify staff
Projects
None yet
Development

No branches or pull requests