1506838 - ASR Snippets: Send to device template issues

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect, P1)

Description

[Giorgos Logiotatidis [:giorgos]]

Attachment: scene1.png

Send to device in 64.0b8 has some major issues with missing default values, wrong parsing of input and display of snippet


  - scene1_button_label no default value. Must default to Learn More.
  -  scene2_dismiss_button_text no default value - Must default to Dismiss
  -  scene2_button_label has default value - Must default to Send
  -  scene2_input_placeholder has default value - Must default to YOUR EMAIL HERE
  -  scene2_title is not displayed when set
  -  scene2_text is not rich text
  -  locale - must default to en-us
  -  country - must default o us
  -  newsletters must default to '' (empty string, not undefined)
  -  include_sms values are True and False as strings. must default to False
  -  Buttons appear larger and text smaller in the new template compared to the old one. (see screenshot. Current template above, new ASR template bellow)
  -  Space of scene2_icon is wrong (see screenshot. Current template above, new ASR template bellow)
  -  Snippet container width is smaller in the new template (see screenshot. Current template above, new ASR template bellow)
  -  Overall the old templates look nicer compared to the old ones (see screenshot. Current template above, new ASR template bellow)

Templates must match what we have for the current AS system so we can automatically migrate the hundreds of active Snippets. 

Ensuring that fields work (i.e. can be populated) and default values are set will shield a proper migration to ASR. 

I haven't completed testing all implemented templates. Same issues may be applicable to other templates as well. Can you please spend some time to QA currently implemented templates?

I'm available to help interpret current templates and answer any questions if needed.

See also https://github.com/mozmeao/snippets-service/issues/831

Comment 1

[Giorgos Logiotatidis [:giorgos]]

Attachment: scene2.png

Comment 2

[[github robot]]

Commit pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/def46f10a13ddac157aea8bd6bd02b0f69e21740
Fix Bug 1506838 - Update Send To Device snippet to better match old template (#4565)

Comment 3

[Ed Lee :Mardak]

https://hg.mozilla.org/mozilla-central/rev/945c7e99ee1f

Comment 4

[Kate Hudson :k88hudson]

Attachment: Bug 1506838 - ASR Snippets: Send to device, template issues

Comment 5

[Kate Hudson :k88hudson]

Comment on attachment 9027011 [details]
Bug 1506838 - ASR Snippets: Send to device, template issues

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1506838

User impact if declined: Some snippets templates will not have appropriate default values for many existing onboarding campaigns, impacting newsletter and mobile campaigns

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: Yes

If yes, steps to reproduce: Please verify with the Newsletter and Send to Device section of https://docs.google.com/document/d/1U8QegwAIXcm3pkznL0mvZ2r0tNXSsnbu9rH82c2dNyQ/edit#heading=h.z267kg6mluya

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Patch has been in nightly for some time; mostly changes to templates, no large structural code changes

String changes made/needed:

Comment 6

[Julien Cristau [:jcristau]]

(In reply to Kate Hudson :k88hudson from comment #5)
> Feature/Bug causing the regression: Bug 1506838

This doesn't answer the question.  When was the issue introduced?

Comment 7

[Kate Hudson :k88hudson]

Apologies, the issue was introduced in Bug 1492088 in which the Send to Device template was implemented.

Comment 8

[Kate Hudson :k88hudson]

I added some steps to the document specifically to verify the default values: https://docs.google.com/document/d/1U8QegwAIXcm3pkznL0mvZ2r0tNXSsnbu9rH82c2dNyQ/edit#heading=h.6q5fw3q1g2zt

Comment 9

[Ryan VanderMeulen [:RyanVM]]

This needs a rebased patch for Beta uplift.

Comment 10

[Marius Coman [:mcoman], Ecosystem QA]

I have verified this issue with the latest "Firefox Nightly" build (65.0a1 Build ID - 20181126100057) installed, on Windows 10 x64, Arch Linux and Mac 10.13.3, and I can confirm the following:
- The snippet's button from the first scene has the "Learn More" text label.
- The submit button from scene two has the "Send" text label written with white chars on a blue background.
- There is a "Dismiss" button displayed in the scene two part of the snippet.
- The text placeholder from the email field is "Your email here" by default.
- The "The intended recipient of the email must have consented. Learn more." string with a hyperlink on "Learn more" part is displayed under the email field.

Comment 11

[Kate Hudson :k88hudson]

Ok, rebased

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9027011 [details]
Bug 1506838 - ASR Snippets: Send to device, template issues

[Triage Comment]
Ensures that snippets templates have the correct default values for various upcoming campaigns. Approved for 64.0b13.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/efca407c5be1

Comment 14

[Marius Coman [:mcoman], Ecosystem QA]

I have verified this issue with the latest "Firefox Beta" build (64.0b13 Build ID - 20181126173133) installed, on Windows 10 x64, Arch Linux and Mac 10.13.3, and I can confirm the following:
- The snippet's button from the first scene has the "Learn More" text label.
- The submit button from scene two has the "Send" text label written with white chars on a blue background.
- There is a "Dismiss" button displayed in the scene two part of the snippet.
- The text placeholder from the email field is "Your email here" by default.
- The "The intended recipient of the email must have consented. Learn more." string with a hyperlink on "Learn more" part is displayed under the email field.