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.

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

MNT Remove TODO comments #1413

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 0 additions & 1 deletion client/src/components/UploadField/UploadField.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,6 @@ class UploadField extends Component {
return;
}

// @todo - Should successful uploads be moved to another state?
this.props.actions.uploadField.succeedUpload(this.props.id, file._queuedId, json[0]);
}

Expand Down
1 change: 0 additions & 1 deletion client/src/components/UploadField/UploadFieldItem.scss
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ $uploadfield-item-label-height: 40px;

// ACTIONS
// Hidden checkbox is controlled via it's label
// @todo - need this?
.uploadfield-item__remove-btn {
margin: 0;
}
Expand Down
13 changes: 1 addition & 12 deletions client/src/containers/AssetAdmin/AssetAdmin.js
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,6 @@ class AssetAdmin extends Component {
this.handleOpenFile(response.record.id);
}

// TODO Update GraphQL store with new model,
// see https://github.com/silverstripe/silverstripe-graphql/issues/14
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return this.props.actions.files.readFiles()
.then(() => {
// open the containing folder, since folder edit mode isn't desired
Expand Down Expand Up @@ -477,9 +475,6 @@ class AssetAdmin extends Component {
*/
handleUnpublish(fileIds) {
return this.doUnpublish(fileIds).then((response) => {
// TODO Update GraphQL store with new model or update apollo and use new API
// see https://github.com/silverstripe/silverstripe-graphql/issues/14
// see https://dev-blog.apollodata.com/apollo-clients-new-imperative-store-api-6cb69318a1e3
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const { fileId } = this.props;
this.props.actions.files.readFiles()
.then(() => {
Expand Down Expand Up @@ -536,8 +531,7 @@ class AssetAdmin extends Component {
}

handleUpload() {
// TODO Update GraphQL store with new model,
// see https://github.com/silverstripe/silverstripe-graphql/issues/14
// noop
}

handleUploadQueue() {
Expand All @@ -558,7 +552,6 @@ class AssetAdmin extends Component {
}

handleMoveFilesSuccess(folderId, fileIds) {
// TODO Refactor "queued files" into separate visual area and remove coupling here
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const files = this.props.queuedFiles.items.filter((file) => (
fileIds.includes(file.id)
));
Expand All @@ -570,8 +563,6 @@ class AssetAdmin extends Component {
});

this.props.actions.gallery.deselectFiles();
// TODO Update GraphQL store with new model,
// see https://github.com/silverstripe/silverstripe-graphql/issues/14
this.props.actions.files.readFiles();
}

Expand Down Expand Up @@ -799,7 +790,6 @@ function mapStateToProps(state, ownProps) {
const { formSchema } = state.assetAdmin.modal;
return {
securityId: state.config.SecurityID,
// TODO Refactor "queued files" into separate visual area and remove coupling here
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

queuedFiles: state.assetAdmin.queuedFiles,
showSearch: state.assetAdmin.displaySearch.isOpen,
type: (formSchema && formSchema.type) || ownProps.type,
Expand All @@ -812,7 +802,6 @@ function mapDispatchToProps(dispatch) {
gallery: bindActionCreators(galleryActions, dispatch),
toasts: bindActionCreators(toastsActions, dispatch),
displaySearch: bindActionCreators(displaySearchActions, dispatch),
// TODO Refactor "queued files" into separate visual area and remove coupling here
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

queuedFiles: bindActionCreators(queuedFilesActions, dispatch),
confirmDeletion: bindActionCreators(confirmDeletionActions, dispatch)
},
Expand Down
1 change: 0 additions & 1 deletion client/src/containers/Editor/LegacyPopoverField.scss
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@

// Unable to use classes within the popover, so we use elements to style
ul {
// TODO remove important by removing nesting of lists
padding-left: 0 !important;
list-style-type: none;
margin-left: -#{$popover-body-padding-x} + 1px; // minus border
Expand Down
6 changes: 0 additions & 6 deletions client/src/containers/Gallery/Gallery.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,27 +127,23 @@ class Gallery extends Component {
}

if (filters.lastEditedFrom && filters.lastEditedTo) {
// TODO Date localisation
messages.push(i18n._t(
'AssetAdmin.SEARCHRESULTSMESSAGEEDITEDBETWEEN',
'last edited between \'{lastEditedFrom}\' and \'{lastEditedTo}\''
));
} else if (filters.lastEditedFrom) {
// TODO Date localisation
messages.push(i18n._t(
'AssetAdmin.SEARCHRESULTSMESSAGEEDITEDFROM',
'last edited after \'{lastEditedFrom}\''
));
} else if (filters.lastEditedTo) {
// TODO Date localisation
messages.push(i18n._t(
'AssetAdmin.SEARCHRESULTSMESSAGEEDITEDTO',
'last edited before \'{lastEditedTo}\''
));
}

if (filters.appCategory) {
// TODO Category name localisation
messages.push(i18n._t(
'AssetAdmin.SEARCHRESULTSMESSAGECATEGORY',
'categorised as \'{appCategory}\''
Expand Down Expand Up @@ -402,8 +398,6 @@ class Gallery extends Component {

this.props.actions.queuedFiles.succeedUpload(fileXhr._queuedId, json[0]);

// TODO Update GraphQL store with new model,
// see https://github.com/silverstripe/silverstripe-graphql/issues/14
if (this.props.onSuccessfulUpload) {
this.props.onSuccessfulUpload(json);
}
Expand Down
1 change: 0 additions & 1 deletion client/src/containers/HistoryList/HistoryItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ class HistoryItem extends Component {
}

return (
// @todo wrap the contents in an `<a>` tag
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions
<li
className="list-group-item history-item"
Expand Down
5 changes: 1 addition & 4 deletions client/src/containers/HistoryList/HistoryList.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ const sectionConfigKey = 'SilverStripe\\AssetAdmin\\Controller\\AssetAdmin';
/**
* Create a new endpoint
*
* @todo duplication with assetadmin.
*
* @param {Object} endpointConfig
* @param {Boolean} includeToken
* @returns {Function}
Expand Down Expand Up @@ -47,7 +45,7 @@ class HistoryList extends Component {
}

componentDidUpdate(prevProps) {
// TODO race conditions happening, this should have history state shifted to redux
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Avoids race conditions from happening
this.refreshHistoryIfNeeded(prevProps);
}

Expand Down Expand Up @@ -75,7 +73,6 @@ class HistoryList extends Component {
* This needs a delay/throttle, so this api request tries to be made last in the stack.
* We also use this to stop an API call happening if the component is going to
* unmount soon.
* TODO: This could potentially be solved by using apollo-client's caching and graphql.
*/
this.timer = setTimeout(() => {
this.api({
Expand Down
1 change: 0 additions & 1 deletion client/src/containers/TableView/TableView.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ class TableView extends Component {
columnMetadata: this.getColumnConfig(),
useGriddleStyles: false,
onRowClick: this.handleRowClick,
// TODO will need to request upstream to stop their internal sorting to show folders first
results: this.props.files,
customNoDataComponent: this.renderNoItemsNotice,
};
Expand Down
4 changes: 0 additions & 4 deletions client/src/entwine/TinyMCE_ssmedia.js
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,6 @@ jQuery.entwine('ss', ($) => {
_handleInsert(data, file) {
let result = false;
this.setData(Object.assign({}, data, file));

// Sometimes AssetAdmin.js handleSubmitEditor() can't find the file
// @todo Ensure that we always return a file for any valid ID

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// in case of any errors, better to catch them than let them go silent
try {
let category = null;
Expand Down
1 change: 0 additions & 1 deletion client/src/entwine/UploadField/UploadFieldEntwine.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ jQuery.entwine('ss', ($) => {

const UploadField = this.getComponent();

// TODO: rework entwine so that react has control of holder
let root = this.getReactRoot();
if (!root) {
root = createRoot(this.getContainer());
Expand Down
4 changes: 0 additions & 4 deletions client/src/state/files/moveFilesMutation.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@ const config = {
fileIds,
},
update: () => {
// todo:
// Refactor this once Apollo GraphQL adds support
// for invalidating specific queries in the cache.
// Context: https://github.com/silverstripe/silverstripe-asset-admin/issues/809
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

window.ss.apolloClient.resetStore();
}
}),
Expand Down
2 changes: 0 additions & 2 deletions client/src/state/gallery/GalleryReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ const initialState = {
concatenateSelect: false,
loading: false,
/**
* @todo Use lowercase identifiers once we can map them in the silverstripe/graphql module
*
* @type {array} sorters
*/
sorters: [
Expand Down
5 changes: 0 additions & 5 deletions code/Controller/AssetAdmin.php
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,6 @@ public function apiCreateFile(HTTPRequest $request)
return null;
}

// TODO Allow batch uploads
$fileClass = File::get_class_for_file_extension(File::get_file_extension($tmpFile['name']));
/** @var File $file */
$file = Injector::inst()->create($fileClass);
Expand Down Expand Up @@ -570,8 +569,6 @@ protected function getNameGenerator($filename)
}

/**
* @todo Implement on client
*
* @param bool $unlinked
* @return ArrayList
*/
Expand Down Expand Up @@ -899,8 +896,6 @@ public function schema(HTTPRequest $request): HTTPResponse
}

// Get schema for history form
// @todo Eventually all form scaffolding will be based on context rather than record ID
// See https://github.com/silverstripe/silverstripe-framework/issues/6362
$itemID = $request->param('ItemID');
$version = $request->param('OtherItemID');
$form = $this->getFileHistoryForm([
Expand Down
3 changes: 0 additions & 3 deletions code/Extensions/RemoteFileModalExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ public function remoteEditFormSchema(HTTPRequest $request)
} catch (NetworkException | RequestException | InvalidRemoteUrlException $exception) {
$errors = ValidationResult::create()
->addError($exception->getMessage());
// @todo - Don't create dummy form (pass $form = null)
$form = Form::create(null, 'Form', FieldList::create(), FieldList::create());
$code = $exception->getCode();

Expand All @@ -121,8 +120,6 @@ public function remoteEditFormSchema(HTTPRequest $request)
/**
* Generate schema for the given form based on the X-Formschema-Request header value
*
* @todo de-dupe this logic with LeftAndMain::getSchemaResponse()
*
* @param string $schemaID ID for this schema. Required.
* @param Form $form Required for 'state' or 'schema' response
* @param ValidationResult $errors Required for 'error' response
Expand Down
1 change: 0 additions & 1 deletion code/Forms/FileFormFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,6 @@ protected function getFormFields(RequestHandler $controller = null, $formName, $
$fields->unshift(LiteralField::create('UnembedableMessage', $unembedableMsg));
}
}
// @todo move specs to a component/class, so it can update specs when a File is replaced
$fields->insertAfter(
'TitleHeader',
LiteralField::create('FileSpecs', $this->getSpecsMarkup($record))
Expand Down
2 changes: 0 additions & 2 deletions tests/behat/src/FixtureContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ public function iShouldSeeTheForm($not, $id, $timeout = 3)
*/
public function iShouldSeeTheFileStatusFlag()
{
// TODO This should wait for any XHRs and React rendering to finish
$this->getMainContext()->getSession()->wait(
1000,
"window.jQuery && window.jQuery('.editor__status-flag').length > 0"
Expand All @@ -106,7 +105,6 @@ public function iShouldSeeTheFileStatusFlag()
*/
public function iShouldNotSeeTheFileStatusFlag()
{
// TODO Flakey assertion, since the status flag might not be loaded via XHR yet
$page = $this->getMainContext()->getSession()->getPage();
$flag = $page->find('css', '.editor__status-flag');
Assert::assertNull($flag, "File editor status flag should not be present");
Expand Down