Skip to content

Commit f8243ea

Browse files
authored
Merge pull request scratchfoundation#4700 from benjiwheeler/upload-in-place
upload in current project id, rather than creating new project id
2 parents 4aba90c + c064eae commit f8243ea

File tree

6 files changed

+36
-51
lines changed

6 files changed

+36
-51
lines changed

src/components/menu-bar/menu-bar.jsx

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -393,19 +393,17 @@ class MenuBar extends React.Component {
393393
</MenuSection>
394394
)}
395395
<MenuSection>
396-
<SBFileUploader onUpdateProjectTitle={this.props.onUpdateProjectTitle}>
396+
<SBFileUploader
397+
canSave={this.props.canSave}
398+
userOwnsProject={this.props.userOwnsProject}
399+
onUpdateProjectTitle={this.props.onUpdateProjectTitle}
400+
>
397401
{(className, renderFileInput, loadProject) => (
398402
<MenuItem
399403
className={className}
400404
onClick={loadProject}
401405
>
402-
<FormattedMessage
403-
defaultMessage="Load from your computer"
404-
description={
405-
'Menu bar item for uploading a project from your computer'
406-
}
407-
id="gui.menuBar.uploadFromComputer"
408-
/>
406+
{this.props.intl.formatMessage(sharedMessages.loadFromComputerTitle)}
409407
{renderFileInput()}
410408
</MenuItem>
411409
)}
@@ -774,7 +772,7 @@ MenuBar.defaultProps = {
774772
onShare: () => {}
775773
};
776774

777-
const mapStateToProps = state => {
775+
const mapStateToProps = (state, ownProps) => {
778776
const loadingState = state.scratchGui.projectState.loadingState;
779777
const user = state.session && state.session.session && state.session.session.user;
780778
return {
@@ -791,6 +789,8 @@ const mapStateToProps = state => {
791789
projectTitle: state.scratchGui.projectTitle,
792790
sessionExists: state.session && typeof state.session.session !== 'undefined',
793791
username: user ? user.username : null,
792+
userOwnsProject: ownProps.authorUsername && user &&
793+
(ownProps.authorUsername === user.username),
794794
vm: state.scratchGui.vm
795795
};
796796
};

src/containers/sb-file-uploader.jsx

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import sharedMessages from '../lib/shared-messages';
1010
import {
1111
LoadingStates,
1212
getIsLoadingUpload,
13-
getIsShowingWithoutId,
1413
onLoadedProject,
1514
requestProjectUpload
1615
} from '../reducers/project-state';
@@ -92,23 +91,29 @@ class SBFileUploader extends React.Component {
9291
handleChange (e) {
9392
const {
9493
intl,
95-
isShowingWithoutId,
9694
loadingState,
97-
projectChanged
95+
projectChanged,
96+
userOwnsProject
9897
} = this.props;
9998

10099
const thisFileInput = e.target;
101100
if (thisFileInput.files) { // Don't attempt to load if no file was selected
102101
this.fileToUpload = thisFileInput.files[0];
103102

104-
// Allow upload to continue only after confirmation if the project
105-
// has changed and is not showing with ID. If it has an ID, this operation
106-
// does not currently overwrite that project, so it is safe to do without confirmation.
107-
const uploadAllowed = (isShowingWithoutId && projectChanged) ?
108-
confirm(intl.formatMessage(sharedMessages.replaceProjectWarning)) : // eslint-disable-line no-alert
109-
true;
110-
111-
if (uploadAllowed) this.props.requestProjectUpload(loadingState);
103+
// If user owns the project, or user has changed the project,
104+
// we must confirm with the user that they really intend to replace it.
105+
// (If they don't own the project and haven't changed it, no need to confirm.)
106+
let uploadAllowed = true;
107+
if (userOwnsProject || projectChanged) {
108+
uploadAllowed = confirm( // eslint-disable-line no-alert
109+
intl.formatMessage(sharedMessages.replaceProjectWarning)
110+
);
111+
}
112+
if (uploadAllowed) {
113+
this.props.requestProjectUpload(loadingState);
114+
} else {
115+
this.props.closeFileMenu();
116+
}
112117
}
113118
}
114119
// called when file upload raw data is available in the reader
@@ -164,15 +169,16 @@ SBFileUploader.propTypes = {
164169
canSave: PropTypes.bool, // eslint-disable-line react/no-unused-prop-types
165170
children: PropTypes.func,
166171
className: PropTypes.string,
172+
closeFileMenu: PropTypes.func,
167173
intl: intlShape.isRequired,
168174
isLoadingUpload: PropTypes.bool,
169-
isShowingWithoutId: PropTypes.bool,
170175
loadingState: PropTypes.oneOf(LoadingStates),
171176
onLoadingFinished: PropTypes.func,
172177
onLoadingStarted: PropTypes.func,
173178
onUpdateProjectTitle: PropTypes.func,
174179
projectChanged: PropTypes.bool,
175180
requestProjectUpload: PropTypes.func,
181+
userOwnsProject: PropTypes.bool,
176182
vm: PropTypes.shape({
177183
loadProject: PropTypes.func
178184
})
@@ -184,14 +190,14 @@ const mapStateToProps = state => {
184190
const loadingState = state.scratchGui.projectState.loadingState;
185191
return {
186192
isLoadingUpload: getIsLoadingUpload(loadingState),
187-
isShowingWithoutId: getIsShowingWithoutId(loadingState),
188193
loadingState: loadingState,
189194
projectChanged: state.scratchGui.projectChanged,
190195
vm: state.scratchGui.vm
191196
};
192197
};
193198

194199
const mapDispatchToProps = (dispatch, ownProps) => ({
200+
closeFileMenu: () => dispatch(closeFileMenu()),
195201
onLoadingFinished: (loadingState, success) => {
196202
dispatch(onLoadedProject(loadingState, ownProps.canSave, success));
197203
dispatch(closeLoadingProject());

src/lib/shared-messages.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,10 @@ export default defineMessages({
2525
id: 'gui.sharedMessages.replaceProjectWarning',
2626
defaultMessage: 'Replace contents of the current project?',
2727
description: 'Confirmation that user wants to overwrite the current project contents'
28+
},
29+
loadFromComputerTitle: {
30+
id: 'gui.sharedMessages.loadFromComputerTitle',
31+
defaultMessage: 'Load from your computer',
32+
description: 'Title for uploading a project from your computer'
2833
}
2934
});

src/reducers/project-state.js

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ const DONE_LOADING_VM_WITHOUT_ID = 'scratch-gui/project-state/DONE_LOADING_VM_WI
1010
const DONE_REMIXING = 'scratch-gui/project-state/DONE_REMIXING';
1111
const DONE_UPDATING = 'scratch-gui/project-state/DONE_UPDATING';
1212
const DONE_UPDATING_BEFORE_COPY = 'scratch-gui/project-state/DONE_UPDATING_BEFORE_COPY';
13-
const DONE_UPDATING_BEFORE_FILE_UPLOAD = 'scratch-gui/project-state/DONE_UPDATING_BEFORE_FILE_UPLOAD';
1413
const DONE_UPDATING_BEFORE_NEW = 'scratch-gui/project-state/DONE_UPDATING_BEFORE_NEW';
1514
const RETURN_TO_SHOWING = 'scratch-gui/project-state/RETURN_TO_SHOWING';
1615
const SET_PROJECT_ID = 'scratch-gui/project-state/SET_PROJECT_ID';
@@ -23,7 +22,6 @@ const START_MANUAL_UPDATING = 'scratch-gui/project-state/START_MANUAL_UPDATING';
2322
const START_REMIXING = 'scratch-gui/project-state/START_REMIXING';
2423
const START_UPDATING_BEFORE_CREATING_COPY = 'scratch-gui/project-state/START_UPDATING_BEFORE_CREATING_COPY';
2524
const START_UPDATING_BEFORE_CREATING_NEW = 'scratch-gui/project-state/START_UPDATING_BEFORE_CREATING_NEW';
26-
const START_UPDATING_BEFORE_FILE_UPLOAD = 'scratch-gui/project-state/START_UPDATING_BEFORE_FILE_UPLOAD';
2725

2826
const defaultProjectId = '0'; // hardcoded id of default project
2927

@@ -43,7 +41,6 @@ const LoadingState = keyMirror({
4341
SHOWING_WITH_ID: null,
4442
SHOWING_WITHOUT_ID: null,
4543
UPDATING_BEFORE_COPY: null,
46-
UPDATING_BEFORE_FILE_UPLOAD: null,
4744
UPDATING_BEFORE_NEW: null
4845
});
4946

@@ -91,7 +88,6 @@ const getIsUpdating = loadingState => (
9188
loadingState === LoadingState.AUTO_UPDATING ||
9289
loadingState === LoadingState.MANUAL_UPDATING ||
9390
loadingState === LoadingState.UPDATING_BEFORE_COPY ||
94-
loadingState === LoadingState.UPDATING_BEFORE_FILE_UPLOAD ||
9591
loadingState === LoadingState.UPDATING_BEFORE_NEW
9692
);
9793
const getIsShowingProject = loadingState => (
@@ -203,13 +199,6 @@ const reducer = function (state, action) {
203199
});
204200
}
205201
return state;
206-
case DONE_UPDATING_BEFORE_FILE_UPLOAD:
207-
if (state.loadingState === LoadingState.UPDATING_BEFORE_FILE_UPLOAD) {
208-
return Object.assign({}, state, {
209-
loadingState: LoadingState.LOADING_VM_FILE_UPLOAD
210-
});
211-
}
212-
return state;
213202
case DONE_UPDATING_BEFORE_NEW:
214203
if (state.loadingState === LoadingState.UPDATING_BEFORE_NEW) {
215204
return Object.assign({}, state, {
@@ -333,13 +322,6 @@ const reducer = function (state, action) {
333322
});
334323
}
335324
return state;
336-
case START_UPDATING_BEFORE_FILE_UPLOAD:
337-
if (state.loadingState === LoadingState.SHOWING_WITH_ID) {
338-
return Object.assign({}, state, {
339-
loadingState: LoadingState.UPDATING_BEFORE_FILE_UPLOAD
340-
});
341-
}
342-
return state;
343325
case START_ERROR:
344326
// fatal errors: there's no correct editor state for us to show
345327
if ([
@@ -360,7 +342,6 @@ const reducer = function (state, action) {
360342
LoadingState.MANUAL_UPDATING,
361343
LoadingState.REMIXING,
362344
LoadingState.UPDATING_BEFORE_COPY,
363-
LoadingState.UPDATING_BEFORE_FILE_UPLOAD,
364345
LoadingState.UPDATING_BEFORE_NEW
365346
].includes(state.loadingState)) {
366347
return Object.assign({}, state, {
@@ -471,10 +452,6 @@ const doneUpdatingProject = loadingState => {
471452
return {
472453
type: DONE_UPDATING_BEFORE_COPY
473454
};
474-
case LoadingState.UPDATING_BEFORE_FILE_UPLOAD:
475-
return {
476-
type: DONE_UPDATING_BEFORE_FILE_UPLOAD
477-
};
478455
case LoadingState.UPDATING_BEFORE_NEW:
479456
return {
480457
type: DONE_UPDATING_BEFORE_NEW
@@ -501,11 +478,8 @@ const requestNewProject = needSave => {
501478

502479
const requestProjectUpload = loadingState => {
503480
switch (loadingState) {
504-
case LoadingState.SHOWING_WITH_ID:
505-
return {
506-
type: START_UPDATING_BEFORE_FILE_UPLOAD
507-
};
508481
case LoadingState.NOT_LOADED:
482+
case LoadingState.SHOWING_WITH_ID:
509483
case LoadingState.SHOWING_WITHOUT_ID:
510484
return {
511485
type: START_LOADING_VM_FILE_UPLOAD

test/integration/menu-bar.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ describe('Menu bar settings', () => {
4141
'//div[contains(@class, "menu-bar_menu-bar-item") and ' +
4242
'contains(@class, "menu-bar_hoverable")][span[text()="File"]]'
4343
);
44-
await findByXpath('//*[li[span[text()="Load from your computer"]] and not(@data-tip="tooltip")]');
44+
await findByXpath('//*[li[text()="Load from your computer"] and not(@data-tip="tooltip")]');
4545
});
4646

4747
test('File->Save should be enabled', async () => {

test/unit/reducers/project-state-reducer.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ test('requestProjectUpload when showing project with id should load', () => {
250250
};
251251
const action = requestProjectUpload(initialState.loadingState);
252252
const resultState = projectStateReducer(initialState, action);
253-
expect(resultState.loadingState).toBe(LoadingState.UPDATING_BEFORE_FILE_UPLOAD);
253+
expect(resultState.loadingState).toBe(LoadingState.LOADING_VM_FILE_UPLOAD);
254254
});
255255

256256
test('requestProjectUpload when showing project without id should load', () => {

0 commit comments

Comments
 (0)