Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Disable exporting functions with duplicate names #33530

Merged
merged 2 commits into from
Mar 10, 2020
Merged

Disable exporting functions with duplicate names #33530

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
75ad9bd
Disable function export if there are 2+ functions with that same name
Mar 9, 2020
281ead1
Unit test disabling input for functions with duplicate names
Mar 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/i18n/common/en_us.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -861,6 +861,7 @@
"libraryCodeError": "We can't publish your library because there is an error in the code. Go look for the square red error indicator and fix the errors.",
"libraryCreatorError": "There was an error creating your library. Contact support@code.org to resolve the issue.",
"libraryDescriptionPlaceholder": "Write a description of your library",
"libraryExportDuplicationFunctionError": "This function cannot be exported because there are multiple functions with this name.",
"libraryExportNoCommentError": "This function cannot be exported until you add a comment to it.",
"libraryExportSubtitle": "Share functions in your project to others in your class or to anyone with the project's ID. Others can import your functions into their projects by going to \"Manage Libraries\" in the gear icon at the top of the toolbox.",
"libraryExportTitle": "Export Functions as a Library",
Expand Down
14 changes: 12 additions & 2 deletions apps/src/code-studio/components/libraries/LibraryPublisher.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,12 @@ export class LibraryPublisher extends React.Component {
const {sourceFunctionList} = this.props.libraryDetails;
return sourceFunctionList.map(sourceFunction => {
const {functionName, comment} = sourceFunction;
const shouldDisable = comment.length === 0;
const noComment = comment.length === 0;
const duplicateFunction =
sourceFunctionList.filter(
source => source.functionName === functionName
).length > 1;
const shouldDisable = noComment || duplicateFunction;
let checked = selectedFunctions[functionName] || false;
if (shouldDisable && checked) {
checked = false;
Expand All @@ -211,9 +216,14 @@ export class LibraryPublisher extends React.Component {
/>
<span>{functionName}</span>
<br />
{shouldDisable && (
{noComment && (
<p style={styles.alert}>{i18n.libraryExportNoCommentError()}</p>
)}
{duplicateFunction && (
<p style={styles.alert}>
{i18n.libraryExportDuplicationFunctionError()}
</p>
)}
<pre style={styles.textInput}>{comment}</pre>
</div>
);
Expand Down
16 changes: 16 additions & 0 deletions apps/test/unit/code-studio/components/libraries/LibraryPublisherTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,22 @@ describe('LibraryPublisher', () => {
expect(checkboxes.last().prop('disabled')).to.be.true;
});

it('disables checkbox for functions with duplicate names', () => {
libraryDetails.sourceFunctionList = libraryDetails.sourceFunctionList.concat(
[
{functionName: 'duplicate', comment: 'first dup!'},
{functionName: 'duplicate', comment: 'another dup!'}
]
);
let wrapper = shallow(
<LibraryPublisher {...DEFAULT_PROPS} libraryDetails={libraryDetails} />
);

let checkboxes = wrapper.find(CHECKBOX_SELECTOR);
expect(checkboxes.at(0).prop('disabled')).to.be.false;
expect(checkboxes.at(1).prop('disabled')).to.be.true;
});

it('checks checkboxes of selected functions', () => {
libraryDetails.sourceFunctionList.push({
functionName: 'bar',
Expand Down