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
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
MNT Remove TODO comments
Sabina Talipova committed Oct 25, 2023
commit 78399879f0432eaf72627754217f7a9b244c071f
1 change: 0 additions & 1 deletion client/src/components/UploadField/UploadField.js
Original file line number Diff line number Diff line change
@@ -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]);
}

1 change: 0 additions & 1 deletion client/src/components/UploadField/UploadFieldItem.scss
Original file line number Diff line number Diff line change
@@ -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;
}
13 changes: 1 addition & 12 deletions client/src/containers/AssetAdmin/AssetAdmin.js
Original file line number Diff line number Diff line change
@@ -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
@@ -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(() => {
@@ -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() {
@@ -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)
));
@@ -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();
}

@@ -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,
@@ -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)
},
1 change: 0 additions & 1 deletion client/src/containers/Editor/LegacyPopoverField.scss
Original file line number Diff line number Diff line change
@@ -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
6 changes: 0 additions & 6 deletions client/src/containers/Gallery/Gallery.js
Original file line number Diff line number Diff line change
@@ -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}\''
@@ -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);
}
1 change: 0 additions & 1 deletion client/src/containers/HistoryList/HistoryItem.js
Original file line number Diff line number Diff line change
@@ -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"
5 changes: 1 addition & 4 deletions client/src/containers/HistoryList/HistoryList.js
Original file line number Diff line number Diff line change
@@ -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}
@@ -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);
}

@@ -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({
1 change: 0 additions & 1 deletion client/src/containers/TableView/TableView.js
Original file line number Diff line number Diff line change
@@ -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,
};
4 changes: 0 additions & 4 deletions client/src/entwine/TinyMCE_ssmedia.js
Original file line number Diff line number Diff line change
@@ -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;
1 change: 0 additions & 1 deletion client/src/entwine/UploadField/UploadFieldEntwine.js
Original file line number Diff line number Diff line change
@@ -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());
4 changes: 0 additions & 4 deletions client/src/state/files/moveFilesMutation.js
Original file line number Diff line number Diff line change
@@ -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();
}
}),
2 changes: 0 additions & 2 deletions client/src/state/gallery/GalleryReducer.js
Original file line number Diff line number Diff line change
@@ -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: [
5 changes: 0 additions & 5 deletions code/Controller/AssetAdmin.php
Original file line number Diff line number Diff line change
@@ -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);
@@ -570,8 +569,6 @@ protected function getNameGenerator($filename)
}

/**
* @todo Implement on client
*
* @param bool $unlinked
* @return ArrayList
*/
@@ -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([
3 changes: 0 additions & 3 deletions code/Extensions/RemoteFileModalExtension.php
Original file line number Diff line number Diff line change
@@ -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();

@@ -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
1 change: 0 additions & 1 deletion code/Forms/FileFormFactory.php
Original file line number Diff line number Diff line change
@@ -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))
2 changes: 0 additions & 2 deletions tests/behat/src/FixtureContext.php
Original file line number Diff line number Diff line change
@@ -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"
@@ -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");