Skip to content

Commit 35c1989

Browse files
iconixiconix
committed
Simplify setLastBadRating(...) -> badRatingAlreadyOccurred()
1 parent bf54d50 commit 35c1989

File tree

4 files changed

+34
-120
lines changed

4 files changed

+34
-120
lines changed

‎src/scripts/clipperUI/panels/ratingsPanel.tsx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,15 @@ class RatingsPanelClass extends ComponentBase<RatingsPanelState, ClipperStatePro
9898
id: Constants.Ids.ratingsButtonInitNo,
9999
label: Localization.getLocalizedString("WebClipper.Label.Ratings.Button.Init.Negative"),
100100
handler: () => {
101-
let lastSeenVersion: string = Clipper.getCachedValue(ClipperStorageKeys.lastSeenVersion);
102-
let badRatingAlreadyOccurred: boolean = RatingsHelper.setLastBadRating(Date.now().toString(), lastSeenVersion);
103-
if (badRatingAlreadyOccurred) {
101+
if (RatingsHelper.badRatingAlreadyOccurred()) {
104102
// setting this to prevent additional ratings prompts after the second bad rating
105103
RatingsHelper.setDoNotPromptStatus();
106104
}
105+
106+
let lastSeenVersion: string = Clipper.getCachedValue(ClipperStorageKeys.lastSeenVersion);
107+
Clipper.storeValue(ClipperStorageKeys.lastBadRatingDate, Date.now().toString());
108+
Clipper.storeValue(ClipperStorageKeys.lastBadRatingVersion, lastSeenVersion);
109+
107110
let feedbackUrl: string = RatingsHelper.getFeedbackUrlIfExists(clipperState);
108111
if (feedbackUrl) {
109112
panel.setState({

‎src/scripts/clipperUI/ratingsHelper.ts‎

Lines changed: 17 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,19 @@ export class RatingsHelper {
3535
public static rateUrlSettingNameSuffix = "_RatingUrl";
3636
public static ratingsPromptEnabledSettingNameSuffix = "_RatingsEnabled";
3737

38+
/**
39+
* Returns true if ClipperStorageKeys.lastBadRatingDate already contained a cached value
40+
* (meaning the user had already rated us negatively)
41+
*/
42+
public static badRatingAlreadyOccurred(): boolean {
43+
let lastBadRatingDateAsStr: string = Clipper.getCachedValue(ClipperStorageKeys.lastBadRatingDate);
44+
let lastBadRatingDate: number = parseInt(lastBadRatingDateAsStr, 10);
45+
if (!isNaN(lastBadRatingDate) && RatingsHelper.isValidDate(lastBadRatingDate)) {
46+
return true;
47+
}
48+
return false;
49+
}
50+
3851
/**
3952
* Get the feedback URL with the special ratings prompt log category, if it exists
4053
*/
@@ -76,51 +89,13 @@ export class RatingsHelper {
7689
Clipper.logger.logEvent(new Log.Event.BaseEvent(Log.Event.Label.SetDoNotPromptRatings));
7790
}
7891

79-
/**
80-
* Sets StorageKeys.lastBadRatingDate to the time provided (if valid),
81-
* and StorageKeys.lastBadRatingVersion to the version provided (if in accepted format).
82-
* Returns true if StorageKeys.lastBadRatingDate already contained a value before this set
83-
* (meaning the user had already rated us negatively)
84-
*/
85-
public static setLastBadRating(badRatingDateToSetAsStr: string, badRatingVersionToSetAsStr: string): boolean {
86-
// TODO decouple, stop returning boolean from here
87-
88-
let badDateKey: string = ClipperStorageKeys.lastBadRatingDate;
89-
let badRatingAlreadyOccurred = false;
90-
91-
let badRatingDateToSet: number = parseInt(badRatingDateToSetAsStr, 10);
92-
if (!RatingsHelper.isValidDate(badRatingDateToSet)) {
93-
// invalid value: err on the side of caution, always set do not prompt status
94-
return true;
95-
}
96-
97-
let badRatingVersionToSet: Version;
98-
try {
99-
badRatingVersionToSet = new Version(badRatingVersionToSetAsStr);
100-
} catch (e) {
101-
// invalid value: err on the side of caution, always set do not prompt status
102-
return true;
103-
}
104-
105-
let lastBadRatingDateAsStr: string = Clipper.getCachedValue(badDateKey);
106-
let lastBadRatingDate: number = parseInt(lastBadRatingDateAsStr, 10);
107-
if (!isNaN(lastBadRatingDate) && RatingsHelper.isValidDate(lastBadRatingDate)) {
108-
badRatingAlreadyOccurred = true;
109-
}
110-
111-
Clipper.storeValue(badDateKey, badRatingDateToSetAsStr);
112-
Clipper.storeValue(ClipperStorageKeys.lastBadRatingVersion, badRatingVersionToSet.toString());
113-
114-
return badRatingAlreadyOccurred;
115-
}
116-
11792
/**
11893
* We will show the ratings prompt if ALL of the below applies:
11994
* * Ratings prompt is enabled for the ClientType/ClipperType
120-
* * If StorageKeys.doNotPromptRatings is not "true"
121-
* * If RatingsHelper.badRatingTimingDelayIsOver(...) returns true when provided StorageKeys.lastBadRatingDate
122-
* * If RatingsHelper.badRatingVersionDelayIsOver(...) returns true when provided StorageKeys.lastBadRatingVersion and StorageKeys.lastSeenVersion
123-
* * If RatingsHelper.clipSuccessDelayIsOver(...) returns true when provided StorageKeys.numClipSuccess
95+
* * If ClipperStorageKeys.doNotPromptRatings is not "true"
96+
* * If RatingsHelper.badRatingTimingDelayIsOver(...) returns true when provided ClipperStorageKeys.lastBadRatingDate
97+
* * If RatingsHelper.badRatingVersionDelayIsOver(...) returns true when provided ClipperStorageKeys.lastBadRatingVersion and ClipperStorageKeys.lastSeenVersion
98+
* * If RatingsHelper.clipSuccessDelayIsOver(...) returns true when provided ClipperStorageKeys.numClipSuccess
12499
*/
125100
public static shouldShowRatingsPrompt(clipperState: ClipperState): boolean {
126101
let shouldShowRatingsPromptEvent = new Log.Event.PromiseEvent(Log.Event.Label.ShouldShowRatingsPrompt);

‎src/tests/clipperUI/panels/ratingsPanel_tests.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ test("'Negative' click at RatingsPromptStage.Init without a prior bad rating goe
143143
strictEqual(RatingsPromptStage[controllerInstance.state.userSelectedRatingsPromptStage], RatingsPromptStage[RatingsPromptStage.Feedback]);
144144

145145
Clipper.getStoredValue(ClipperStorageKeys.doNotPromptRatings, (doNotPromptRatingsAsStr: string) => {
146-
strictEqual(doNotPromptRatingsAsStr, undefined);
146+
strictEqual(doNotPromptRatingsAsStr, undefined, "doNotPromptRatings should be undefined");
147147
done();
148148
});
149149
});
@@ -170,7 +170,7 @@ test("'Negative' click at RatingsPromptStage.Init without a prior bad rating goe
170170
strictEqual(RatingsPromptStage[controllerInstance.state.userSelectedRatingsPromptStage], RatingsPromptStage[RatingsPromptStage.End]);
171171

172172
Clipper.getStoredValue(ClipperStorageKeys.doNotPromptRatings, (doNotPromptRatingsAsStr: string) => {
173-
strictEqual(doNotPromptRatingsAsStr, undefined);
173+
strictEqual(doNotPromptRatingsAsStr, undefined, "doNotPromptRatings should be undefined");
174174
done();
175175
});
176176
});

‎src/tests/clipperUI/ratingsHelper_tests.tsx‎

Lines changed: 9 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -362,95 +362,31 @@ test("ratingsPromptEnabledForClient returns true if a client's enable value is '
362362
strictEqual(isEnabled, true);
363363
});
364364

365-
// setLastBadRating
365+
// badRatingAlreadyOccurred
366366

367-
test("setLastBadRating sets lastBadRatingDate in storage and returns false when lastBadRatingDate does not exist in storage", (assert: QUnitAssert) => {
368-
let done = assert.async();
369-
370-
let badRatingDateToSet: string = Date.now().toString();
371-
let badRatingVersionToSet = "3.0.0";
372-
373-
let alreadyRatedBad: boolean = RatingsHelper.setLastBadRating(badRatingDateToSet, badRatingVersionToSet);
367+
test("badRatingAlreadyOccurred returns false when lastBadRatingDate does not exist in storage", () => {
368+
let alreadyRatedBad: boolean = RatingsHelper.badRatingAlreadyOccurred();
374369
strictEqual(alreadyRatedBad, false);
375-
Clipper.getStoredValue(ClipperStorageKeys.lastBadRatingDate, (badRateDateAsStr: string) => {
376-
Clipper.getStoredValue(ClipperStorageKeys.lastBadRatingVersion, (badRateVersionAsStr: string) => {
377-
strictEqual(badRateDateAsStr, badRatingDateToSet, "bad rating date is incorrect");
378-
strictEqual(badRateVersionAsStr, badRatingVersionToSet, "bad rating version is incorrect");
379-
done();
380-
});
381-
});
382370
});
383371

384-
test("setLastBadRating sets lastBadRatingDate in storage and returns false when lastBadRatingDate is not a number", (assert: QUnitAssert) => {
385-
let done = assert.async();
386-
372+
test("badRatingAlreadyOccurred returns false when lastBadRatingDate is not a number", () => {
387373
Clipper.storeValue(ClipperStorageKeys.lastBadRatingDate, "not a number");
388374

389-
let badRatingDateToSet: string = Date.now().toString();
390-
let badRatingVersionToSet = "3.0.0";
391-
392-
let alreadyRatedBad: boolean = RatingsHelper.setLastBadRating(badRatingDateToSet, badRatingVersionToSet);
375+
let alreadyRatedBad: boolean = RatingsHelper.badRatingAlreadyOccurred();
393376
strictEqual(alreadyRatedBad, false);
394-
Clipper.getStoredValue(ClipperStorageKeys.lastBadRatingDate, (badRateDateAsStr: string) => {
395-
Clipper.getStoredValue(ClipperStorageKeys.lastBadRatingVersion, (badRateVersionAsStr: string) => {
396-
strictEqual(badRateDateAsStr, badRatingDateToSet, "bad rating date is incorrect");
397-
strictEqual(badRateVersionAsStr, badRatingVersionToSet, "bad rating version is incorrect");
398-
done();
399-
});
400-
});
401377
});
402378

403-
test("setLastBadRating sets lastBadRatingDate in storage and returns false when lastBadRatingDate is a number out of date range", (assert: QUnitAssert) => {
404-
let done = assert.async();
405-
379+
test("badRatingAlreadyOccurred returns false when lastBadRatingDate is a number out of date range", () => {
406380
Clipper.storeValue(ClipperStorageKeys.lastBadRatingDate, (Constants.Settings.maximumJSTimeValue + 1).toString());
407381

408-
let badRatingDateToSet: string = Date.now().toString();
409-
let badRatingVersionToSet = "3.0.0";
410-
411-
let alreadyRatedBad: boolean = RatingsHelper.setLastBadRating(badRatingDateToSet, badRatingVersionToSet);
382+
let alreadyRatedBad: boolean = RatingsHelper.badRatingAlreadyOccurred();
412383
strictEqual(alreadyRatedBad, false);
413-
Clipper.getStoredValue(ClipperStorageKeys.lastBadRatingDate, (badRateDateAsStr: string) => {
414-
Clipper.getStoredValue(ClipperStorageKeys.lastBadRatingVersion, (badRateVersionAsStr: string) => {
415-
strictEqual(badRateDateAsStr, badRatingDateToSet, "bad rating date is incorrect");
416-
strictEqual(badRateVersionAsStr, badRatingVersionToSet, "bad rating version is incorrect");
417-
done();
418-
});
419-
});
420384
});
421385

422-
test("setLastBadRating sets lastBadRatingDate in storage and returns true when lastBadRatingDate is a valid number", (assert: QUnitAssert) => {
423-
let done = assert.async();
424-
386+
test("badRatingAlreadyOccurred returns true when lastBadRatingDate is a valid number", () => {
425387
Clipper.storeValue(ClipperStorageKeys.lastBadRatingDate, Date.now().toString());
426388

427-
let badRatingDateToSet: string = Date.now().toString();
428-
let badRatingVersionToSet = "3.0.0";
429-
430-
let alreadyRatedBad: boolean = RatingsHelper.setLastBadRating(badRatingDateToSet, badRatingVersionToSet);
431-
strictEqual(alreadyRatedBad, true);
432-
Clipper.getStoredValue(ClipperStorageKeys.lastBadRatingDate, (badRateDateAsStr: string) => {
433-
Clipper.getStoredValue(ClipperStorageKeys.lastBadRatingVersion, (badRateVersionAsStr: string) => {
434-
strictEqual(badRateDateAsStr, badRatingDateToSet, "bad rating date is incorrect");
435-
strictEqual(badRateVersionAsStr, badRatingVersionToSet, "bad rating version is incorrect");
436-
done();
437-
});
438-
});
439-
});
440-
441-
test("setLastBadRating rejects when badRatingDateToSet is not a valid date", () => {
442-
let badRatingDateToSet: string = (Constants.Settings.maximumJSTimeValue + 1).toString();
443-
let badRatingVersionToSet = "3.0.0";
444-
445-
let alreadyRatedBad: boolean = RatingsHelper.setLastBadRating(badRatingDateToSet, badRatingVersionToSet);
446-
strictEqual(alreadyRatedBad, true);
447-
});
448-
449-
test("setLastBadRating rejects when badRatingVersionToSet is not in a valid version format", () => {
450-
let badRatingDateToSet: string = Date.now().toString();
451-
let badRatingVersionToSet = "12345";
452-
453-
let alreadyRatedBad: boolean = RatingsHelper.setLastBadRating(badRatingDateToSet, badRatingVersionToSet);
389+
let alreadyRatedBad: boolean = RatingsHelper.badRatingAlreadyOccurred();
454390
strictEqual(alreadyRatedBad, true);
455391
});
456392

0 commit comments

Comments
 (0)