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

Support: Truncate the log text so it fits in the ticket field. #12980

Merged
merged 2 commits into from Nov 20, 2019

Conversation

@ScoutHarris
Copy link
Contributor

ScoutHarris commented Nov 20, 2019

Fixes #12297

As noted on #12297 , the problem was:

When any request is created, we automatically take the Current activity log and populate the System Status Report field with it. If the log is big enough, it will exceed the field limit. This will cause any request to fail (even if you're not attaching anything).

The limit on that field type isn't specified, and I got no response from Zendesk support. So I did some testing, and found that it'll handle about 50k characters. So that's the limit.

If the log text exceeds that limit, the text is truncated from the beginning so we get the last 50k characters (i.e. most recent).

To test:
On an account with a large Current activity log:

  • Go to Me > Help & Support > Contact us.
  • Attempt to send a message.
  • Verify it sends.
  • Bonus:
    • Find your ticket in Zendesk
    • Verify the System Status Report field has content.
    • Verify the last log message is the most recent.

Update release notes:

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.
@ScoutHarris ScoutHarris added the Support label Nov 20, 2019
@ScoutHarris ScoutHarris added this to the 13.8 milestone Nov 20, 2019
@ScoutHarris ScoutHarris requested review from designsimply and yaelirub Nov 20, 2019
@ScoutHarris ScoutHarris self-assigned this Nov 20, 2019
@peril-wordpress-mobile

This comment has been minimized.

Copy link

peril-wordpress-mobile bot commented Nov 20, 2019

You can trigger an installable build for these changes by visiting CircleCI here.

@yaelirub

This comment has been minimized.

Copy link
Contributor

yaelirub commented Nov 20, 2019

@ScoutHarris , thank you for working on this! I'm going to test it now. Quick question: Should we add a unit test for the log limit?

@yaelirub

This comment has been minimized.

Copy link
Contributor

yaelirub commented Nov 20, 2019

Tested and works as expected. LGTM, I only have 1 comment for adding unit testing if possible

@ScoutHarris

This comment has been minimized.

Copy link
Contributor Author

ScoutHarris commented Nov 20, 2019

Hey @yaelirub . We could. I'd rather do that separately though. I suspect we could add unit tests for other ZD related items as well.

@yaelirub

This comment has been minimized.

Copy link
Contributor

yaelirub commented Nov 20, 2019

Oh but why not add it here just for the log size?
We recently added this check to the description of tickets we create in the team to make sure we considered adding a unit test, example:

  • If it's feasible, I have added unit tests — unfortunately, AztecPostViewController is big and hard to test.
@ScoutHarris

This comment has been minimized.

Copy link
Contributor Author

ScoutHarris commented Nov 20, 2019

I'm not saying it's not worth doing, I'd just rather do another PR for that. And that check wasn't there when I created this PR so....grandfathered in! 😄

@yaelirub

This comment has been minimized.

Copy link
Contributor

yaelirub commented Nov 20, 2019

Ok. Feel free to add me as a reviewer to the following PR 🙏

Copy link
Contributor

yaelirub left a comment

LGTM since Unit test will be added in a following PR

@ScoutHarris

This comment has been minimized.

Copy link
Contributor Author

ScoutHarris commented Nov 20, 2019

Thanks @yaelirub !

@ScoutHarris ScoutHarris merged commit 929aa42 into develop Nov 20, 2019
7 checks passed
7 checks passed
Hound No violations found. Woof!
Peril All green. Good on 'ya.
Details
ci/circleci: Build Tests Your tests passed on CircleCI!
Details
ci/circleci: Installable Build/Hold Your job is on hold on CircleCI!
Details
ci/circleci: UI Tests (iPad Air 3rd generation) Your tests passed on CircleCI!
Details
ci/circleci: UI Tests (iPhone 11) Your tests passed on CircleCI!
Details
ci/circleci: Unit Tests Your tests passed on CircleCI!
Details
@ScoutHarris ScoutHarris deleted the fix/12297-support_message_fail branch Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.