Closed Bug 1080661 Opened 7 years ago Closed 7 years ago

The count of shared URLs should be increased only once per generated URL

Categories

(Hello (Loop) :: Client, defect, P1)

defect
Points:
1

Tracking

(firefox34 verified, firefox35 verified)

VERIFIED FIXED
mozilla35
Iteration:
35.3
Tracking Status
firefox34 --- verified
firefox35 --- verified

People

(Reporter: Paolo, Assigned: Paolo)

References

Details

(Whiteboard: [loop-uplift])

Attachments

(1 file)

Currently, the count of shared URLs is increased every time the "copy" or "email" buttons are used, or the link is copied manually. This, however, should only happen once per generated URL, even if different ways of sharing are used.
Attached patch The patchSplinter Review
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=572fd15ad4f2

In addition to automated unit testing, manual testing should verify that when a new URL is generated and shared after the first one, the shared count is increased.

I kept the patch simple to facilitate uplift, so this scenario isn't unit-tested, since it is difficult to reach the internal code that resets the flag.
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Attachment #8502587 - Flags: review?(MattN+bmo)
Flags: qe-verify?
Flags: firefox-backlog+
Flags: qe-verify? → qe-verify+
Comment on attachment 8502587 [details] [diff] [review]
The patch

Review of attachment 8502587 [details] [diff] [review]:
-----------------------------------------------------------------

The fix seems reasonable.

(In reply to :Paolo Amadini from comment #1)
> I kept the patch simple to facilitate uplift, so this scenario isn't
> unit-tested, since it is difficult to reach the internal code that resets
> the flag.

This is unfortunate as it seems like something fairly easy to break.
Attachment #8502587 - Flags: review?(MattN+bmo) → review+
Blocks: 1080732
(In reply to Matthew N. [:MattN] (behind on reviews - focused on Loop) from comment #2)
> (In reply to :Paolo Amadini from comment #1)
> > I kept the patch simple to facilitate uplift, so this scenario isn't
> > unit-tested, since it is difficult to reach the internal code that resets
> > the flag.
> 
> This is unfortunate as it seems like something fairly easy to break.

I agree. We should have a recurring manual test on file for now, and I filed bug 1080732 about adding the automated test.
https://hg.mozilla.org/mozilla-central/rev/86abd23340f0
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Hi Pauly, This should fix the bug you found yesterday in bug 1015988.  Can you verify?  Thanks!
Flags: needinfo?(paul.silaghi)
Verified fixed 35.0a1 (2014-10-10) Win 7, Ubuntu 13.04, OS X 10.9.5
Status: RESOLVED → VERIFIED
Flags: needinfo?(paul.silaghi)
Flags: needinfo?(paul.silaghi)
Verified fixed FF 34b1 Win 7
Flags: needinfo?(paul.silaghi)
You need to log in before you can comment on or make changes to this bug.