Replace shot expiration with favorites for FxA users (#4726)

* Replace shot expiration with favorites for FxA users. (#4688) * Fix server tests that require an account id. (#4688)
mozilla-services · Aug 20, 2018 · 46e8b73 · 46e8b73
1 parent db64484
commit 46e8b73
Show file tree

Hide file tree

Showing 13 changed files with 129 additions and 128 deletions.
diff --git a/bin/require.pip b/bin/require.pip
@@ -3,3 +3,4 @@ GitPython==2.1.3
 requests==2.18.2
 sarge==0.1.4
 pytest==3.2.1
+psycopg2==2.7.5
diff --git a/docs/METRICS.md b/docs/METRICS.md
@@ -290,11 +290,6 @@ These are events that an add-on user can encounter on a shot they own
   8. [x] Confirm delete from shot index `web/delete/my-shots-popup-confirm`
   9. [x] Cancel delete from shot index, `web/cancel-delete/my-shots-popup-confirm`
 7. [x] Click My Shots `web/goto-myshots/navbar`
-11. [x] Try to change expiration time `web/start-expiration-change/navbar`
-  12. [x] Cancel changing expiration time `web/cancel-expiration-change/navbar`
-  13. [x] Change expiration time `web/set-expiration/navbar`
-  14. [x] Change expiration time to specific time `web/set-expiration-to-time/navbar`
-  15. [x] Change expiration time to indefinite `web/set-expiration-to-indefinite/navbar`
 16. [x] View expired shot `web/view-expired/owner`
 17. [x] Recover expired shot `web/recover-expired`
 19. [x] Visit original page `web/view-original/navbar-owner`
@@ -316,6 +311,8 @@ These are events that an add-on user can encounter on a shot they own
 33. [x] Visit an image directly, when the image isn't embedded directly in a Screenshots shot page, `web/visit/direct-view-owner`
 34. [x] View an image directly, when the image is being shown as part of a Facebook/Twitter style preview (the og:image or twitter:image), `web/visit/direct-view-embedded-owner`
 35. [x] Close new edit tools promotion dialog, `web/promo-closed`
+1. [x] Add shot to favorites `web/set-favorite/navbar`
+1. [x] Remove shot from favorites `web-unset-favorite/navbar`
 
 #### Shot Index (My Shots)
 

diff --git a/locales/en-US/server.ftl b/locales/en-US/server.ftl
@@ -289,6 +289,11 @@ shotIndexPageNextPage =
 # language/culture).
 shotIndexNoExpirationSymbol = ∞
     .title = This shot does not expire
+# This is the tooltip for a "heart" symbol in the lower right corner of the
+# card for a shot on the My Shots page. It indicate that the shot was marked as
+# a favorite by the owner.
+shotIndexFavoriteIcon =
+    .title = This is a favorite shot and it does not expire
 
 
 ## Delete Confirmation Dialog

diff --git a/server/src/pages/shot/model.js b/server/src/pages/shot/model.js
@@ -15,6 +15,7 @@ exports.createModel = function(req) {
   }
   const title = req.getText("shotPageTitle", {originalTitle: req.shot.title});
   const enableAnnotations = req.config.enableAnnotations;
+  const isFxaAuthenticated = req.accountId && req.accountId === req.shot.accountId;
   const serverPayload = {
     title,
     staticLink: req.staticLink,
@@ -25,7 +26,8 @@ exports.createModel = function(req) {
     id: req.shot.id,
     productName: req.config.productName,
     isExtInstalled: !!req.deviceId,
-    isOwner: req.deviceId === req.shot.ownerId || (req.accountId && req.accountId === req.shot.accountId),
+    isOwner: req.deviceId === req.shot.ownerId || isFxaAuthenticated,
+    isFxaAuthenticated,
     gaId: req.config.gaId,
     deviceId: req.deviceId,
     authenticated: !!req.deviceId,
@@ -54,7 +56,8 @@ exports.createModel = function(req) {
     id: req.shot.id,
     productName: req.config.productName,
     isExtInstalled: !!req.deviceId,
-    isOwner: req.deviceId === req.shot.ownerId || (req.accountId && req.accountId === req.shot.accountId),
+    isOwner: req.deviceId === req.shot.ownerId || isFxaAuthenticated,
+    isFxaAuthenticated,
     gaId: req.config.gaId,
     deviceId: req.deviceId,
     authenticated: !!req.deviceId,

diff --git a/server/src/pages/shot/view.js b/server/src/pages/shot/view.js
@@ -176,7 +176,6 @@ class Body extends React.Component {
     this.state = {
       hidden: false,
       closeBanner: false,
-      isChangingExpire: false,
       imageEditing: false
     };
   }
@@ -342,19 +341,21 @@ class Body extends React.Component {
     const linkTextShort = shot.urlDisplay;
 
     const timeDiff = <TimeDiff date={shot.createdDate} />;
-    let expiresDiff = null;
-    if (this.props.isOwner) {
-      expiresDiff = <span className="expire-widget">
-      <ExpireWidget
-        expireTime={this.props.expireTime}
-        onChanging={this.onChangingExpire.bind(this)}
-        onSaveExpire={this.onSaveExpire.bind(this)} />
-      </span>;
-    }
 
+    let favoriteShotButton = null;
     let trashOrFlagButton;
     let editButton;
     const highlight = this.state.highlightEditButton ? <div className="edit-highlight" onClick={ this.onClickEdit.bind(this) } onMouseOver={ this.onMouseOverHighlight.bind(this) } onMouseOut={ this.onMouseOutHighlight.bind(this) }></div> : null;
+
+    if (this.props.isFxaAuthenticated) {
+      const activeFavClass = this.props.expireTime ? "" : "is-fav";
+      favoriteShotButton = <div className="fav-wrapper"><button
+        className={`button favorite ${activeFavClass}`}
+        title="Favorite this shot"
+        onClick={ this.onClickFavorite.bind(this) }
+      /></div>;
+    }
+
     if (this.props.isOwner) {
       trashOrFlagButton = <DeleteShotButton
         clickDeleteHandler={ this.clickDeleteHandler.bind(this) }
@@ -419,13 +420,13 @@ class Body extends React.Component {
             <div className="shot-info">
               <EditableTitle title={shot.title} isOwner={this.props.isOwner} />
               <div className="shot-subtitle">
-                { linkTextShort && !this.state.isChangingExpire ? <a className="subtitle-link" rel="noopener noreferrer" href={ shot.url } target="_blank" onClick={ this.onClickOrigUrl.bind(this, "navbar") }>{ linkTextShort }</a> : null }
-                { this.state.isChangingExpire ? null : <span className="time-diff">{ timeDiff }</span> }
-                { expiresDiff }
+                { linkTextShort ? <a className="subtitle-link" rel="noopener noreferrer" href={ shot.url } target="_blank" onClick={ this.onClickOrigUrl.bind(this, "navbar") }>{ linkTextShort }</a> : null }
+                <span className="time-diff">{ timeDiff }</span>
               </div>
             </div>
           </div>
           <div className="shot-alt-actions">
+            { favoriteShotButton }
             { trashOrFlagButton }
             { this.props.enableAnnotations ? editButton : null }
             { highlight }
@@ -508,20 +509,6 @@ class Body extends React.Component {
     sendEvent("click-install-firefox-shot", {useBeacon: true});
   }
 
-  onSaveExpire(value) {
-    sendEvent("set-expiration", "navbar");
-    if (value === 0 || value === "0") {
-      sendEvent("set-expiration-to-indefinite", "navbar");
-    } else {
-      sendEvent("set-expiration-to-time", "navbar");
-    }
-    this.props.controller.changeShotExpiration(this.props.shot, value);
-  }
-
-  onChangingExpire(value) {
-    this.setState({isChangingExpire: value});
-  }
-
   onRestore() {
     sendEvent("recover-expired");
     this.props.controller.changeShotExpiration(this.props.shot, this.props.defaultExpiration);
@@ -545,6 +532,18 @@ class Body extends React.Component {
     // Note: we allow the default action to continue
   }
 
+  onClickFavorite() {
+    if (this.props.expireTime) {
+      sendEvent("set-favorite", "navbar");
+      const INDEFINITE = 0;
+      this.props.controller.changeShotExpiration(this.props.shot, INDEFINITE);
+    } else {
+      sendEvent("unset-favorite", "navbar");
+      const TWO_WEEKS_IN_MS = 1209600000;
+      this.props.controller.changeShotExpiration(this.props.shot, Date.now() + TWO_WEEKS_IN_MS);
+    }
+  }
+
   onClickDownload() {
     sendEvent("download", "navbar");
   }
@@ -570,6 +569,7 @@ Body.propTypes = {
   isExtInstalled: PropTypes.bool,
   isMobile: PropTypes.bool,
   isOwner: PropTypes.bool,
+  isFxaAuthenticated: PropTypes.bool,
   loginFailed: PropTypes.bool,
   pngToJpegCutoff: PropTypes.number,
   retentionTime: PropTypes.number,
@@ -580,96 +580,6 @@ Body.propTypes = {
   userLocales: PropTypes.array
 };
 
-class ExpireWidget extends React.Component {
-
-  constructor(props) {
-    super(props);
-    this.state = {isChangingExpire: false};
-  }
-
-  render() {
-    if (this.state.isChangingExpire) {
-      return this.renderChanging();
-    }
-    return this.renderNormal();
-  }
-
-  renderChanging() {
-    const minute = 60 * 1000;
-    const hour = minute * 60;
-    const day = hour * 24;
-    return (
-      <span className="keep-for-form">
-        <Localized id="shotPageKeepFor"><span>How long should this shot be retained?</span></Localized>
-        <select ref={expireTime => this.expireTime = expireTime}>
-          <Localized id="shotPageSelectTime"><option value="cancel">Select time</option></Localized>
-          <Localized id="shotPageKeepIndefinitelyWithSymbol"><option value="0">Indefinitely &infin;</option></Localized>
-          <Localized id="shotPageKeepTenMinutes"><option value={ 10 * minute }>10 Minutes</option></Localized>
-          <Localized id="shotPageKeepOneHour"><option value={ hour }>1 Hour</option></Localized>
-          <Localized id="shotPageKeepOneDay"><option value={ day }>1 Day</option></Localized>
-          <Localized id="shotPageKeepOneWeek"><option value={ 7 * day }>1 Week</option></Localized>
-          <Localized id="shotPageKeepTwoWeeks"><option value={ 14 * day }>2 Weeks</option></Localized>
-          <Localized id="shotPageKeepOneMonth"><option value={ 31 * day }>1 Month</option></Localized>
-        </select>
-        <Localized id="shotPageSaveExpiration"><span className="button tiny secondary" tabIndex="0" onClick={this.clickSaveExpire.bind(this)}>save</span></Localized>
-        <Localized id="shotPageCancelExpiration"><span className="button tiny secondary" tabIndex="0" onClick={this.clickCancelExpire.bind(this)}>cancel</span></Localized>
-      </span>
-    );
-  }
-
-  renderNormal() {
-    let button;
-    if (this.props.expireTime === null) {
-      button = <Localized id="shotPageDoesNotExpire"><span>does not expire</span></Localized>;
-    } else {
-      const expired = this.props.expireTime < Date.now();
-      const timediff = <TimeDiff date={this.props.expireTime} simple={this.props.simple} />;
-      if (expired) {
-        button = <Localized id="shotPageExpired" $timediff={timediff}><span>expired {timediff}</span></Localized>;
-      } else {
-        button = <Localized id="shotPageExpiresIn" $timediff={timediff}><span>expires {timediff}</span></Localized>;
-      }
-    }
-    return (
-      <button className="button tiny secondary inline" onClick={this.clickChangeExpire.bind(this)}>
-        {button}
-      </button>
-    );
-  }
-
-  clickChangeExpire() {
-    sendEvent("start-expiration-change", "navbar");
-    this.setState({isChangingExpire: true});
-    this.props.onChanging(true);
-  }
-
-  clickCancelExpire() {
-    sendEvent("cancel-expiration-change", "navbar");
-    this.setState({isChangingExpire: false});
-    this.props.onChanging(false);
-  }
-
-  clickSaveExpire() {
-    // FIXME: save the value that it was changed to?  Yes!  Not sure where to put it.
-    let value = this.expireTime.value;
-    if (value === "cancel") {
-      this.clickCancelExpire();
-      return;
-    }
-    value = parseInt(value, 10);
-    // Note: sendEvent done in onSaveExpire
-    this.props.onSaveExpire(value);
-    this.props.onChanging(false);
-    this.setState({isChangingExpire: false});
-  }
-}
-
-ExpireWidget.propTypes = {
-  expireTime: PropTypes.number,
-  onChanging: PropTypes.func,
-  onSaveExpire: PropTypes.func,
-  simple: PropTypes.bool
-};
 
 class EditableTitle extends React.Component {
 

diff --git a/server/src/pages/shotindex/model.js b/server/src/pages/shotindex/model.js
@@ -8,6 +8,7 @@ exports.createModel = function(req) {
   }
   const serverModel = {
     title,
+    hasFxa: !!req.accountId,
     hasDeviceId: req.deviceId !== undefined,
     defaultSearch: query || null
   };

diff --git a/server/src/pages/shotindex/view.js b/server/src/pages/shotindex/view.js
@@ -63,7 +63,7 @@ class Body extends React.Component {
     const children = [];
     if (this.props.shots && this.props.shots.length) {
       for (const shot of this.props.shots) {
-        children.push(<Card shot={shot} downloadUrl={this.props.downloadUrls[shot.id]} abTests={this.props.abTests} clipUrl={shot.urlDisplay} isOwner={this.props.isOwner} staticLink={this.props.staticLink} isExtInstalled={this.props.isExtInstalled} key={shot.id} />);
+        children.push(<Card shot={shot} downloadUrl={this.props.downloadUrls[shot.id]} abTests={this.props.abTests} clipUrl={shot.urlDisplay} isOwner={this.props.isOwner} staticLink={this.props.staticLink} isExtInstalled={this.props.isExtInstalled} hasFxa={this.props.hasFxa} key={shot.id} />);
       }
     }
 
@@ -281,6 +281,7 @@ Body.propTypes = {
   downloadUrls: PropTypes.object,
   enableUserSettings: PropTypes.bool,
   hasDeviceId: PropTypes.bool,
+  hasFxa: PropTypes.bool,
   isExtInstalled: PropTypes.bool,
   isOwner: PropTypes.bool,
   pageNumber: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
@@ -329,7 +330,11 @@ class Card extends React.Component {
 
     let neverExpireIndicator = null;
     if (!shot.expireTime) {
-      neverExpireIndicator = <Localized id="shotIndexNoExpirationSymbol"><div className="never-expires" title=""></div></Localized>;
+      if (this.props.hasFxa) {
+        neverExpireIndicator = <Localized id="shotIndexFavoriteIcon"><div className="favorite-shot" title=""></div></Localized>;
+      } else {
+        neverExpireIndicator = <Localized id="shotIndexNoExpirationSymbol"><div className="never-expires" title=""></div></Localized>;
+      }
     }
 
     const deleteConfirmationClass = this.state.deletePanelOpen ? "panel-open" : "";
@@ -462,6 +467,7 @@ class Card extends React.Component {
 Card.propTypes = {
   abTests: PropTypes.object,
   downloadUrl: PropTypes.string,
+  hasFxa: PropTypes.bool,
   isExtInstalled: PropTypes.bool,
   isOwner: PropTypes.bool,
   shot: PropTypes.object,

diff --git a/server/src/server.js b/server/src/server.js
@@ -861,7 +861,7 @@ app.post("/api/save-edit", function(req, res) {
 });
 
 app.post("/api/set-expiration", function(req, res) {
-  if (!req.deviceId) {
+  if (!req.deviceId || !req.accountId) {
     sendRavenMessage(req, "Attempt to set expiration without login");
     simpleResponse(res, "Not logged in", 401);
     return;

diff --git a/static/css/frame.scss b/static/css/frame.scss
@@ -128,6 +128,26 @@
   }
 }
 
+.fav-wrapper {
+  &:active,
+  &:hover {
+    background-color: $light-hover;
+  }
+  border-radius: $border-radius;
+  margin-right: $grid-unit * 0.5;
+}
+.favorite {
+  background: url("../img/icon-heart.svg");
+  background-size: 20px 20px;
+  background-position: center;
+  background-repeat: no-repeat;
+  filter: brightness(2.4);
+
+  &.is-fav {
+    filter: brightness(1);
+  }
+}
+
 .delete-confirmation-dialog {
   &.right-align {
     top: 48px;
@@ -567,7 +587,7 @@ body {
   height: 40px;
   border-radius: 3px;
   position: absolute;
-  background-color: #ededf0;
+  background-color: $light-hover;
 }
 
 #color-picker {

diff --git a/static/css/shot-index.scss b/static/css/shot-index.scss
@@ -156,6 +156,7 @@ h1 {
     .alt-actions-container {
       opacity: 1;
     }
+    .favorite-shot,
     .never-expires {
       display: none;
     }
@@ -244,13 +245,23 @@ h1 {
     margin: 12px;
   }
 
+  .favorite-shot,
   .never-expires {
     bottom: 0;
     color: #9e9e9e;
     font-size: 1.2em;
     position: absolute;
     right: 8px;
   }
+
+  .favorite-shot {
+    background-image: url("../img/icon-heart.svg");
+    background-position: center;
+    background-repeat: no-repeat;
+    background-size: 16px 16px;
+    width: 16px;
+    height: 26px;
+  }
 }
 
 .preferences {

diff --git a/static/img/icon-heart.svg b/static/img/icon-heart.svg