Merge pull request #2660 from LLK/develop

[MASTER] Deploy reply chain fixes and remix fixes
2024-11-23 07:38:07 -05:00 · 2019-01-11 11:23:21 -05:00 · 2019-01-11 11:23:21 -05:00 · ba4fcb9707
commit ba4fcb9707
parent 1153bb21eb 266a46a75b
10 changed files with 145 additions and 49 deletions
--- a/package.json
+++ b/package.json
@ -78,8 +78,10 @@
    "lodash.defaultsdeep": "3.10.0",
    "lodash.isarray": "3.0.4",
    "lodash.merge": "3.3.2",
+    "lodash.mergewith": "4.6.1",
    "lodash.omit": "3.1.0",
    "lodash.range": "3.0.1",
+    "lodash.uniqby": "4.7.0",
    "minilog": "2.0.8",
    "node-dir": "0.1.16",
    "node-sass": "4.6.1",
@ -100,7 +102,7 @@
    "redux": "3.5.2",
    "redux-thunk": "2.0.1",
    "sass-loader": "6.0.6",
-    "scratch-gui": "0.1.0-prerelease.20190109203727",
+    "scratch-gui": "0.1.0-prerelease.20190111141703",
    "scratch-l10n": "latest",
    "scratchr2_translations": "git://github.com/LLK/scratchr2_translations.git#master",
    "slick-carousel": "1.6.0",
--- a/src/l10n.json
+++ b/src/l10n.json
@ -237,7 +237,7 @@
    "comments.post": "Post",
    "comments.cancel": "Cancel",
    "comments.lengthWarning": "{remainingCharacters, plural, one {1 character left} other {{remainingCharacters} characters left}}",
-    "comments.seeMoreReplies": "{repliesCount, plural, one {See 1 more reply} other {See all {repliesCount} replies}}",
+    "comments.loadMoreReplies": "See more replies",
    "comments.status.delbyusr": "Deleted by project owner",
    "comments.status.censbyfilter": "Censored by filter",
    "comments.status.delbyparentcomment": "Parent comment deleted",
--- a/src/redux/preview.js
+++ b/src/redux/preview.js
@ -1,11 +1,14 @@
 const defaults = require('lodash.defaults');
 const keyMirror = require('keymirror');
 const async = require('async');
-const merge = require('lodash.merge');
+const mergeWith = require('lodash.mergewith');
+const uniqBy = require('lodash.uniqby');

 const api = require('../lib/api');
 const log = require('../lib/log');

+const COMMENT_LIMIT = 20;
+
 module.exports.Status = keyMirror({
    FETCHED: null,
    NOT_FETCHED: null,
@ -100,7 +103,7 @@ module.exports.previewReducer = (state, action) => {
        });
    case 'SET_COMMENTS':
        return Object.assign({}, state, {
-            comments: [...state.comments, ...action.items] // TODO: consider a different way of doing this?
+            comments: uniqBy(state.comments.concat(action.items), 'id')
        });
    case 'UPDATE_COMMENT':
        if (action.topLevelCommentId) {
@ -149,7 +152,19 @@ module.exports.previewReducer = (state, action) => {
        });
    case 'SET_REPLIES':
        return Object.assign({}, state, {
-            replies: merge({}, state.replies, action.replies)
+            // Append new replies to the state.replies structure
+            replies: mergeWith({}, state.replies, action.replies, (replies, newReplies) => (
+                uniqBy((replies || []).concat(newReplies || []), 'id')
+            )),
+            // Also set the `moreRepliesToLoad` property on the top-level comments
+            comments: state.comments.map(comment => {
+                if (action.replies[comment.id]) {
+                    return Object.assign({}, comment, {
+                        moreRepliesToLoad: action.replies[comment.id].length === COMMENT_LIMIT
+                    });
+                }
+                return comment;
+            })
        });
    case 'SET_LOVED':
        return Object.assign({}, state, {
@ -448,7 +463,6 @@ module.exports.getFavedStatus = (id, username, token) => (dispatch => {
 });

 module.exports.getTopLevelComments = (id, offset, isAdmin, token) => (dispatch => {
-    const COMMENT_LIMIT = 20;
    dispatch(module.exports.setFetchStatus('comments', module.exports.Status.FETCHING));
    api({
        uri: `${isAdmin ? '/admin' : ''}/projects/${id}/comments`,
@ -467,7 +481,7 @@ module.exports.getTopLevelComments = (id, offset, isAdmin, token) => (dispatch =
        }
        dispatch(module.exports.setFetchStatus('comments', module.exports.Status.FETCHED));
        dispatch(module.exports.setComments(body));
-        dispatch(module.exports.getReplies(id, body.map(comment => comment.id), isAdmin, token));
+        dispatch(module.exports.getReplies(id, body.map(comment => comment.id), 0, isAdmin, token));

        // If we loaded a full page of comments, assume there are more to load.
        // This will be wrong (1 / COMMENT_LIMIT) of the time, but does not require
@ -503,17 +517,18 @@ module.exports.getCommentById = (projectId, commentId, isAdmin, token) => (dispa
        // If the comment is not a reply, show it as top level and load replies
        dispatch(module.exports.setFetchStatus('comments', module.exports.Status.FETCHED));
        dispatch(module.exports.setComments([body]));
-        dispatch(module.exports.getReplies(projectId, [body.id], isAdmin, token));
+        dispatch(module.exports.getReplies(projectId, [body.id], 0, isAdmin, token));
    });
 });

-module.exports.getReplies = (projectId, commentIds, isAdmin, token) => (dispatch => {
+module.exports.getReplies = (projectId, commentIds, offset, isAdmin, token) => (dispatch => {
    dispatch(module.exports.setFetchStatus('replies', module.exports.Status.FETCHING));
    const fetchedReplies = {};
    async.eachLimit(commentIds, 10, (parentId, callback) => {
        api({
            uri: `${isAdmin ? '/admin' : ''}/projects/${projectId}/comments/${parentId}/replies`,
-            authentication: isAdmin ? token : null
+            authentication: isAdmin ? token : null,
+            params: {offset: offset || 0, limit: COMMENT_LIMIT}
        }, (err, body) => {
            if (err) {
                return callback(`Error fetching comment replies: ${err}`);
--- a/src/views/faq/l10n.json
+++ b/src/views/faq/l10n.json
@ -24,7 +24,7 @@
 	"faq.requirementsDesktopSafari":"Safari (11+)",
 	"faq.requirementsDesktopIE":"Internet Explorer is NOT supported.",
 	"faq.requirementsTablet":"Tablet",
-	"faq.requirementsTabletChrome":"Mobile Chrome (62+)",
+	"faq.requirementsTabletChrome":"Mobile Chrome (63+)",
 	"faq.requirementsTabletSafari":"Mobile Safari (11+)",
 	"faq.requirementsNote":"Note:",
 	"faq.requirementsNoteDesktop":"If your computer doesn’t meet these requirements, you can try the <a href=\"/download\">Scratch Desktop</a> editor (see next item in FAQ). ",
--- a/src/views/preview/comment/comment.scss
+++ b/src/views/preview/comment/comment.scss
@ -235,22 +235,20 @@

 .replies {
    width: calc(100% - 4rem);
+}

-    &.collapsed .comment {
-        &:last-child {
-            &:after {
-                position: absolute;
-                bottom: 0;
-                background: linear-gradient(
-                    $ui-light-primary-transparent,
-                    $ui-light-primary
-                );
-                width: 100%;
-                height: 100%;
-                content: "";
-                pointer-events: none;
-            }
-        }
+.replies.collapsed > .comment:last-of-type {
+    &:after {
+        position: absolute;
+        bottom: 0;
+        background: linear-gradient(
+            $ui-light-primary-transparent,
+            $ui-light-primary
+        );
+        width: 100%;
+        height: 100%;
+        content: "";
+        pointer-events: none;
    }
 }

--- a/src/views/preview/comment/top-level-comment.jsx
+++ b/src/views/preview/comment/top-level-comment.jsx
@ -28,9 +28,11 @@ class TopLevelComment extends React.Component {
    }

    handleExpandThread () {
-        this.setState({
-            expanded: true
-        });
+        if (this.state.expanded) {
+            this.props.onLoadMoreReplies(this.props.id, this.props.replies.length);
+        } else {
+            this.setState({expanded: true});
+        }
    }

    handleDeleteReply (replyId) {
@ -79,6 +81,7 @@ class TopLevelComment extends React.Component {
            datetimeCreated,
            highlightedCommentId,
            id,
+            moreRepliesToLoad,
            onDelete,
            onReport,
            onRestore,
@ -141,21 +144,17 @@ class TopLevelComment extends React.Component {
                                onRestore={this.handleRestoreReply}
                            />
                        ))}
+                        {((!this.state.expanded && replies.length > 3) ||
+                            (this.state.expanded && moreRepliesToLoad)) &&
+                            <a
+                                className="expand-thread"
+                                onClick={this.handleExpandThread}
+                            >
+                                <FormattedMessage id="comments.loadMoreReplies" />
+                            </a>
+                        }
                    </FlexRow>
                }
-                {!this.state.expanded && replies.length > 3 &&
-                    <a
-                        className="expand-thread"
-                        onClick={this.handleExpandThread}
-                    >
-                        <FormattedMessage
-                            id="comments.seeMoreReplies"
-                            values={{
-                                repliesCount: replies.length
-                            }}
-                        />
-                    </a>
-                }
            </FlexRow>
        );
    }
@ -178,8 +177,10 @@ TopLevelComment.propTypes = {
    deletable: PropTypes.bool,
    highlightedCommentId: PropTypes.oneOfType([PropTypes.number, PropTypes.bool]),
    id: PropTypes.number,
+    moreRepliesToLoad: PropTypes.bool,
    onAddComment: PropTypes.func,
    onDelete: PropTypes.func,
+    onLoadMoreReplies: PropTypes.func,
    onReport: PropTypes.func,
    onRestore: PropTypes.func,
    parentId: PropTypes.number,
@ -189,7 +190,8 @@ TopLevelComment.propTypes = {
 };

 TopLevelComment.defaultProps = {
-    defaultExpanded: false
+    defaultExpanded: false,
+    moreRepliesToLoad: false
 };

 module.exports = TopLevelComment;
--- a/src/views/preview/presentation.jsx
+++ b/src/views/preview/presentation.jsx
@ -70,6 +70,7 @@ const PreviewPresentation = ({
    isFullScreen,
    isLoggedIn,
    isNewScratcher,
+    isProjectLoaded,
    isRemixing,
    isScratcher,
    isShared,
@ -88,8 +89,10 @@ const PreviewPresentation = ({
    onFavoriteClicked,
    onGreenFlag,
    onLoadMore,
+    onLoadMoreReplies,
    onLoveClicked,
    onOpenAdminPanel,
+    onProjectLoaded,
    onRemix,
    onRemixing,
    onReportClicked,
@ -250,10 +253,11 @@ const PreviewPresentation = ({
                                            className={classNames([
                                                'remix-button',
                                                {
-                                                    remixing: isRemixing,
-                                                    spin: isRemixing
+                                                    disabled: isRemixing || !isProjectLoaded,
+                                                    remixing: isRemixing
                                                }
                                            ])}
+                                            disabled={isRemixing || !isProjectLoaded}
                                            title={intl.formatMessage({id: 'project.remixButton.altText'})}
                                            onClick={onRemix}
                                        >
@ -301,6 +305,7 @@ const PreviewPresentation = ({
                                    projectHost={projectHost}
                                    projectId={projectId}
                                    onGreenFlag={onGreenFlag}
+                                    onProjectLoaded={onProjectLoaded}
                                    onRemixing={onRemixing}
                                    onUpdateProjectId={onUpdateProjectId}
                                    onUpdateProjectThumbnail={onUpdateProjectThumbnail}
@ -550,12 +555,14 @@ const PreviewPresentation = ({
                                                highlightedCommentId={singleCommentId}
                                                id={comment.id}
                                                key={comment.id}
+                                                moreRepliesToLoad={comment.moreRepliesToLoad}
                                                parentId={comment.parent_id}
                                                projectId={projectId}
                                                replies={replies && replies[comment.id] ? replies[comment.id] : []}
                                                visibility={comment.visibility}
                                                onAddComment={onAddComment}
                                                onDelete={onDeleteComment}
+                                                onLoadMoreReplies={onLoadMoreReplies}
                                                onReport={onReportComment}
                                                onRestore={onRestoreComment}
                                            />
@ -623,6 +630,7 @@ PreviewPresentation.propTypes = {
    isFullScreen: PropTypes.bool,
    isLoggedIn: PropTypes.bool,
    isNewScratcher: PropTypes.bool,
+    isProjectLoaded: PropTypes.bool,
    isRemixing: PropTypes.bool,
    isScratcher: PropTypes.bool,
    isShared: PropTypes.bool,
@ -644,8 +652,10 @@ PreviewPresentation.propTypes = {
    onFavoriteClicked: PropTypes.func,
    onGreenFlag: PropTypes.func,
    onLoadMore: PropTypes.func,
+    onLoadMoreReplies: PropTypes.func,
    onLoveClicked: PropTypes.func,
    onOpenAdminPanel: PropTypes.func,
+    onProjectLoaded: PropTypes.func,
    onRemix: PropTypes.func,
    onRemixing: PropTypes.func,
    onReportClicked: PropTypes.func.isRequired,
--- a/src/views/preview/preview.scss
+++ b/src/views/preview/preview.scss
@ -283,9 +283,11 @@ $stage-width: 480px;
            background-image: url("/svgs/project/remix-white.svg");
        }
    }
+    .remix-button.disabled {
+        opacity: .6;
+    }

    .remix-button.remixing {
-        opacity: .6;

        &:before {
            animation-name: remix-intro, remix-spin;
--- a/src/views/preview/project-view.jsx
+++ b/src/views/preview/project-view.jsx
@ -60,6 +60,7 @@ class Preview extends React.Component {
            'handleToggleStudio',
            'handleFavoriteToggle',
            'handleLoadMore',
+            'handleLoadMoreReplies',
            'handleLoveToggle',
            'handleMessage',
            'handlePopState',
@ -74,6 +75,7 @@ class Preview extends React.Component {
            'handleAddToStudioClick',
            'handleAddToStudioClose',
            'handleGreenFlag',
+            'handleProjectLoaded',
            'handleRemix',
            'handleSeeAllComments',
            'handleSeeInside',
@ -108,6 +110,7 @@ class Preview extends React.Component {
            clientLoved: false,
            extensions: [],
            favoriteCount: 0,
+            isProjectLoaded: false,
            isRemixing: false,
            invalidProject: parts.length === 1,
            justRemixed: false,
@ -382,6 +385,11 @@ class Preview extends React.Component {
            this.props.setFullScreen(fullScreen);
        }
    }
+    handleProjectLoaded () {
+        // Currently project view only needs to know when the project becomes loaded. It
+        // does not currently handle (or need to handle) the case where a project becomes unloaded.
+        this.setState({isProjectLoaded: true});
+    }
    pushHistory (push) {
        // update URI to match mode
        const idPath = this.state.projectId ? `${this.state.projectId}/` : '';
@ -440,6 +448,11 @@ class Preview extends React.Component {
        this.props.getTopLevelComments(this.state.projectId, this.props.comments.length,
            this.props.isAdmin, this.props.user && this.props.user.token);
    }
+    handleLoadMoreReplies (commentId, offset) {
+        this.props.getMoreReplies(this.state.projectId, commentId, offset,
+            this.props.isAdmin, this.props.user && this.props.user.token
+        );
+    }
    handleLoveToggle () {
        if (!this.props.lovedLoaded) return;

@ -611,6 +624,7 @@ class Preview extends React.Component {
                            isFullScreen={this.state.isFullScreen}
                            isLoggedIn={this.props.isLoggedIn}
                            isNewScratcher={this.props.isNewScratcher}
+                            isProjectLoaded={this.state.isProjectLoaded}
                            isRemixing={this.state.isRemixing}
                            isScratcher={this.props.isScratcher}
                            isShared={this.props.isShared}
@ -644,8 +658,10 @@ class Preview extends React.Component {
                            onFavoriteClicked={this.handleFavoriteToggle}
                            onGreenFlag={this.handleGreenFlag}
                            onLoadMore={this.handleLoadMore}
+                            onLoadMoreReplies={this.handleLoadMoreReplies}
                            onLoveClicked={this.handleLoveToggle}
                            onOpenAdminPanel={this.handleOpenAdminPanel}
+                            onProjectLoaded={this.handleProjectLoaded}
                            onRemix={this.handleRemix}
                            onRemixing={this.handleIsRemixing}
                            onReportClicked={this.handleReportClick}
@ -691,6 +707,7 @@ class Preview extends React.Component {
                            onGreenFlag={this.handleGreenFlag}
                            onLogOut={this.props.handleLogOut}
                            onOpenRegistration={this.props.handleOpenRegistration}
+                            onProjectLoaded={this.handleProjectLoaded}
                            onRemixing={this.handleIsRemixing}
                            onSetLanguage={this.handleSetLanguage}
                            onShare={this.handleShare}
@ -736,6 +753,7 @@ Preview.propTypes = {
    getCuratedStudios: PropTypes.func.isRequired,
    getFavedStatus: PropTypes.func.isRequired,
    getLovedStatus: PropTypes.func.isRequired,
+    getMoreReplies: PropTypes.func.isRequired,
    getOriginalInfo: PropTypes.func.isRequired,
    getParentInfo: PropTypes.func.isRequired,
    getProjectInfo: PropTypes.func.isRequired,
@ -948,6 +966,9 @@ const mapDispatchToProps = dispatch => ({
    getCommentById: (projectId, commentId, isAdmin, token) => {
        dispatch(previewActions.getCommentById(projectId, commentId, isAdmin, token));
    },
+    getMoreReplies: (projectId, commentId, offset, isAdmin, token) => {
+        dispatch(previewActions.getReplies(projectId, [commentId], offset, isAdmin, token));
+    },
    getFavedStatus: (id, username, token) => {
        dispatch(previewActions.getFavedStatus(id, username, token));
    },
--- a/test/unit/redux/preview-test.js
+++ b/test/unit/redux/preview-test.js
@ -59,9 +59,9 @@ tap.test('setComments', t => {
    // Initial value
    t.deepEqual(initialState.comments, []);

-    state = reducer(initialState, Preview.setComments([1, 2]));
-    state = reducer(state, Preview.setComments([3, 4]));
-    t.deepEqual(state.comments, [1, 2, 3, 4]);
+    state = reducer(initialState, Preview.setComments([{id: 1}, {id: 2}]));
+    state = reducer(state, Preview.setComments([{id: 3}, {id: 4}]));
+    t.deepEqual(state.comments, [{id: 1}, {id: 2}, {id: 3}, {id: 4}]);

    t.end();
 });
@ -80,6 +80,13 @@ const commentState = {
    }
 };

+tap.test('setComments, discards duplicates', t => {
+    state = reducer(commentState, Preview.setComments([{id: 'id1'}]));
+    // Does not increase the number of comments, still 3
+    t.equal(state.comments.length, 3);
+    t.end();
+});
+
 tap.test('setCommentDeleted, top level comment', t => {
    state = reducer(commentState, Preview.setCommentDeleted('id2'));
    t.equal(state.comments[1].visibility, 'deleted');
@ -128,3 +135,42 @@ tap.test('addNewComment, reply comment', t => {
    t.equal(state.replies.id1[2].id, 'new comment');
    t.end();
 });
+
+tap.test('setReplies', t => {
+    // setReplies should append new replies
+    state = reducer(commentState, Preview.setReplies({
+        id1: {id: 'id6'}
+    }));
+    t.equal(state.replies.id1[2].id, 'id6');
+    t.equal(state.comments[0].moreRepliesToLoad, false);
+
+    // setReplies should ignore duplicates, do the same as above again
+    t.equal(state.replies.id1.length, 3);
+    state = reducer(state, Preview.setReplies({id1: {id: 'id6'}}));
+    t.equal(state.replies.id1.length, 3);
+
+    // setReplies can add replies to a comment that didn't have any
+    state = reducer(state, Preview.setReplies({
+        id2: {id: 'id7'}
+    }));
+    t.equal(state.replies.id1.length, 3);
+    t.equal(state.replies.id2.length, 1);
+    t.equal(state.replies.id2[0].id, 'id7');
+    t.equal(state.comments[0].moreRepliesToLoad, false);
+    t.equal(state.comments[1].moreRepliesToLoad, false);
+
+    // Getting 20 (COMMENT_LIMIT) replies sets moreRepliesToLoad to true
+    state = reducer(state, Preview.setReplies({
+        id3: (new Array(20)).map((_, i) => ({id: `id${i + 1}`}))
+    }));
+    t.equal(state.comments[0].moreRepliesToLoad, false);
+    t.equal(state.comments[1].moreRepliesToLoad, false);
+    t.equal(state.comments[2].moreRepliesToLoad, true);
+
+    // Getting one more reply sets moreRepliesToLoad back to false
+    state = reducer(state, Preview.setReplies({
+        id3: {id: 'id21'}
+    }));
+    t.equal(state.comments[2].moreRepliesToLoad, false);
+    t.end();
+});