From 56511c8618e5c831e5611d2cc74c37bed2de697d Mon Sep 17 00:00:00 2001 From: Sabina Talipova Date: Thu, 19 Oct 2023 10:46:23 +1300 Subject: [PATCH 1/2] MNT Remove TODO comments --- sake | 2 -- src/Control/ContentNegotiator.php | 6 ----- src/Control/Controller.php | 2 -- src/Control/CookieJar.php | 4 --- src/Control/HTTP.php | 1 - .../Middleware/HTTPCacheControlMiddleware.php | 2 -- src/Control/RSS/RSSFeed.php | 3 --- src/Control/RequestHandler.php | 2 -- src/Control/SimpleResourceURLGenerator.php | 1 - src/Core/BaseKernel.php | 1 - src/Core/ClassInfo.php | 4 --- src/Core/Convert.php | 1 - src/Core/Extensible.php | 2 -- src/Core/Injector/Injector.php | 4 --- src/Dev/BulkLoader.php | 5 ---- src/Dev/BulkLoader_Result.php | 2 -- src/Dev/CliDebugView.php | 2 -- src/Dev/Constraint/ViewableDataContains.php | 2 -- src/Dev/CsvBulkLoader.php | 9 ------- src/Dev/Debug.php | 4 --- src/Dev/DevelopmentAdmin.php | 5 ---- src/Dev/State/ExtensionTestState.php | 3 --- src/Forms/CheckboxSetField.php | 8 ------ src/Forms/CompositeField.php | 2 -- src/Forms/CurrencyField.php | 2 -- src/Forms/DefaultFormFactory.php | 2 -- src/Forms/FieldGroup.php | 3 --- src/Forms/FieldList.php | 4 --- src/Forms/FormField.php | 9 ------- src/Forms/FormRequestHandler.php | 1 - src/Forms/FormScaffolder.php | 5 ---- src/Forms/GridField/GridField.php | 6 ----- .../GridFieldDetailForm_ItemRequest.php | 8 ------ src/Forms/GridField/GridFieldExportButton.php | 1 - src/Forms/GroupedDropdownField.php | 2 -- src/Forms/MoneyField.php | 1 - src/Forms/SelectionGroup.php | 1 - src/Forms/TimeField.php | 2 -- .../DebugViewFriendlyErrorFormatter.php | 1 - src/Logging/HTTPOutputHandler.php | 2 -- src/ORM/Connect/DBConnector.php | 1 - src/ORM/Connect/DBSchemaManager.php | 5 ---- src/ORM/Connect/Database.php | 2 -- src/ORM/DB.php | 2 -- src/ORM/DataList.php | 9 ------- src/ORM/DataObject.php | 27 ------------------- src/ORM/DataObjectSchema.php | 2 +- src/ORM/DataQuery.php | 6 +---- src/ORM/DatabaseAdmin.php | 2 -- src/ORM/FieldType/DBComposite.php | 2 -- src/ORM/FieldType/DBDatetime.php | 1 - src/ORM/FieldType/DBDouble.php | 1 - src/ORM/FieldType/DBField.php | 8 ------ src/ORM/FieldType/DBLocale.php | 2 -- src/ORM/FieldType/DBMultiEnum.php | 1 - src/ORM/FieldType/DBPolymorphicForeignKey.php | 3 +-- src/ORM/FieldType/DBPrimaryKey.php | 2 -- src/ORM/Filters/ExactMatchFilter.php | 1 - src/ORM/Filters/FulltextFilter.php | 1 - src/ORM/Filters/SearchFilter.php | 1 - src/ORM/Filters/WithinRangeFilter.php | 5 ---- src/ORM/HasManyList.php | 1 - src/ORM/PolymorphicHasManyList.php | 1 - src/ORM/Queries/SQLAssignmentRow.php | 2 -- src/ORM/Queries/SQLConditionalExpression.php | 7 ----- src/ORM/Queries/SQLExpression.php | 2 -- src/ORM/Queries/SQLSelect.php | 1 - src/ORM/Search/SearchContext.php | 6 ----- src/Security/Group.php | 6 ----- src/Security/GroupCsvBulkLoader.php | 3 --- src/Security/Member.php | 7 ----- .../ChangePasswordHandler.php | 1 - .../CookieAuthenticationHandler.php | 4 --- .../MemberAuthenticator.php | 1 - .../MemberAuthenticator/MemberLoginForm.php | 2 -- .../SessionAuthenticationHandler.php | 2 -- src/Security/MemberCsvBulkLoader.php | 1 - src/Security/Permission.php | 1 - src/Security/Security.php | 1 - src/Security/SecurityToken.php | 1 - src/View/SSTemplateParser.php | 7 +---- src/View/ViewableData.php | 2 -- src/i18n/TextCollection/i18nTextCollector.php | 5 ---- tests/behat/src/CmsFormsContext.php | 4 +-- tests/behat/src/CmsUiContext.php | 2 -- tests/php/Control/DirectorTest.php | 3 --- tests/php/Control/SessionTest.php | 3 --- tests/php/Core/ConvertTest.php | 2 -- .../Core/Manifest/ThemeResourceLoaderTest.php | 1 - tests/php/Core/ObjectTest.php | 7 ----- tests/php/Core/PhpSyntaxTest.php | 2 -- tests/php/Dev/CsvBulkLoaderTest.php | 2 -- tests/php/Forms/CurrencyFieldDisabledTest.php | 3 --- tests/php/Forms/CurrencyFieldReadonlyTest.php | 3 --- tests/php/Forms/EmailFieldTest.php | 2 -- tests/php/Forms/FieldListTest.php | 17 ------------ .../GridFieldDetailFormTest/Category.php | 1 - .../GridFieldDetailFormTest/PeopleGroup.php | 1 - .../GridFieldDetailFormTest/Person.php | 1 - tests/php/Forms/GridField/GridFieldTest.php | 4 --- tests/php/Forms/ListboxFieldTest.php | 9 ------- tests/php/Forms/NumericFieldTest.php | 3 --- tests/php/Forms/RequiredFieldsTest.php | 3 --- tests/php/Forms/TimeFieldTest.php | 1 - tests/php/ORM/DBFieldTest.php | 2 -- tests/php/ORM/DataObjectLazyLoadingTest.php | 2 -- tests/php/ORM/DataObjectTest.php | 16 ----------- tests/php/ORM/DataQueryTest.php | 3 --- .../ORM/ManyManyThroughListTest/PolyItem.php | 1 - tests/php/ORM/PolymorphicHasManyListTest.php | 2 -- .../php/Security/MemberCsvBulkLoaderTest.php | 1 - .../php/Security/SecurityDefaultAdminTest.php | 2 -- tests/php/View/RequirementsTest.php | 4 --- tests/php/View/SSViewerTest.php | 1 - 114 files changed, 5 insertions(+), 378 deletions(-) diff --git a/sake b/sake index 3fac1c411..3e1274d8c 100755 --- a/sake +++ b/sake @@ -90,7 +90,6 @@ if [ "$1" = "-start" ]; then url=$3 fi - # TODO: Give a globally unique processname by including the projectname as well processname=$2 daemon -n $processname -r -D $base --pidfile=$pidfile --stdout=$outlog --stderr=$errlog $sake $url @@ -105,7 +104,6 @@ if [ "$1" = "-stop" ]; then if [ -f $pidfile ]; then echo "Stopping service $2" - # TODO: This is a bad way of killing the process kill -KILL `cat $pidfile` unlink $pidfile else diff --git a/src/Control/ContentNegotiator.php b/src/Control/ContentNegotiator.php index 3e17d06d4..21ec89f3e 100644 --- a/src/Control/ContentNegotiator.php +++ b/src/Control/ContentNegotiator.php @@ -25,10 +25,6 @@ use SilverStripe\Core\Injector\Injectable; * * Please see http://webkit.org/blog/68/understanding-html-xml-and-xhtml/ for further information. * - * @todo Check for correct XHTML doctype in xhtml() - * @todo Allow for other HTML4 doctypes (e.g. Transitional) in html() - * @todo Make content replacement and doctype setting two separately configurable behaviours - * * Some developers might know what they're doing and don't want ContentNegotiator messing with their * HTML4 doctypes, but still find it useful to have self-closing tags removed. */ @@ -169,8 +165,6 @@ class ContentNegotiator * , checked, selected). * * @param HTTPResponse $response - * - * @todo Search for more xhtml replacement */ public function xhtml(HTTPResponse $response) { diff --git a/src/Control/Controller.php b/src/Control/Controller.php index da5a67266..80d7047c0 100644 --- a/src/Control/Controller.php +++ b/src/Control/Controller.php @@ -146,8 +146,6 @@ class Controller extends RequestHandler implements TemplateGlobalProvider /** * A bootstrap for the handleRequest method * - * @todo setDataModel and setRequest are redundantly called in parent::handleRequest() - sort this out - * * @param HTTPRequest $request */ protected function beforeHandleRequest(HTTPRequest $request) diff --git a/src/Control/CookieJar.php b/src/Control/CookieJar.php index 0009e09dc..c2667bfcb 100644 --- a/src/Control/CookieJar.php +++ b/src/Control/CookieJar.php @@ -11,10 +11,6 @@ use LogicException; * This backend allows one to better test Cookie setting and separate cookie * handling from the core * - * @todo Create a config array for defaults (eg: httpOnly, secure, path, domain, expiry) - * @todo A getter for cookies that haven't been sent to the browser yet - * @todo Tests / a way to set the state without hacking with $_COOKIE - * @todo Store the meta information around cookie setting (path, domain, secure, etc) */ class CookieJar implements Cookie_Backend { diff --git a/src/Control/HTTP.php b/src/Control/HTTP.php index 0fa24e060..d13bd7728 100644 --- a/src/Control/HTTP.php +++ b/src/Control/HTTP.php @@ -121,7 +121,6 @@ class HTTP $regExps[] = "/(<{$tagPrefix}[^>]*$attrib *= *)([^\"' ]*)( )/i"; } // Replace css styles - // @todo - http://www.css3.info/preview/multiple-backgrounds/ $styles = ['background-image', 'background', 'list-style-image', 'list-style', 'content']; foreach ($styles as $style) { $regExps[] = "/($style:[^;]*url *\\(\")([^\"]+)(\"\\))/i"; diff --git a/src/Control/Middleware/HTTPCacheControlMiddleware.php b/src/Control/Middleware/HTTPCacheControlMiddleware.php index 4c7f0ab5a..94fb0e2c3 100644 --- a/src/Control/Middleware/HTTPCacheControlMiddleware.php +++ b/src/Control/Middleware/HTTPCacheControlMiddleware.php @@ -28,8 +28,6 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable /** * Generate response for the given request * - * @todo Refactor HTTP::add_cache_headers() (e.g. etag handling) into this middleware - * * @param HTTPRequest $request * @param callable $delegate * @return HTTPResponse diff --git a/src/Control/RSS/RSSFeed.php b/src/Control/RSS/RSSFeed.php index 597a1f09a..c397f5454 100644 --- a/src/Control/RSS/RSSFeed.php +++ b/src/Control/RSS/RSSFeed.php @@ -18,7 +18,6 @@ use SilverStripe\View\ViewableData; * RSSFeed class * * This class is used to create an RSS feed. - * @todo Improve documentation */ class RSSFeed extends ViewableData { @@ -215,8 +214,6 @@ class RSSFeed extends ViewableData /** * Output the feed to the browser. * - * TODO: Pass $response object to ->outputToBrowser() to loosen dependence on global state for easier testing/prototyping so dev can inject custom HTTPResponse instance. - * * @return DBHTMLText */ public function outputToBrowser() diff --git a/src/Control/RequestHandler.php b/src/Control/RequestHandler.php index e724f7494..872c6a05b 100644 --- a/src/Control/RequestHandler.php +++ b/src/Control/RequestHandler.php @@ -171,8 +171,6 @@ class RequestHandler extends ViewableData // We used to put "handleAction" as the action on controllers, but (a) this could only be called when // you had $Action in your rule, and (b) RequestHandler didn't have one. $Action is better if ($action == 'handleAction') { - // TODO Fix LeftAndMain usage - // Deprecation::notice('3.2.0', 'Calling handleAction directly is deprecated - use $Action instead'); $action = '$Action'; } diff --git a/src/Control/SimpleResourceURLGenerator.php b/src/Control/SimpleResourceURLGenerator.php index ae4b59cb4..c41f68ef3 100644 --- a/src/Control/SimpleResourceURLGenerator.php +++ b/src/Control/SimpleResourceURLGenerator.php @@ -140,7 +140,6 @@ class SimpleResourceURLGenerator implements ResourceURLGenerator // All resources mapped directly to _resources/ $relativePath = Path::join(RESOURCES_DIR, $relativePath); } elseif (stripos($relativePath ?? '', ManifestFileFinder::VENDOR_DIR . DIRECTORY_SEPARATOR) === 0) { - // @todo Non-public dir support will be removed in 5.0, so remove this block there // If there is no public folder, map to _resources/ but trim leading vendor/ too (4.0 compat) $relativePath = Path::join( RESOURCES_DIR, diff --git a/src/Core/BaseKernel.php b/src/Core/BaseKernel.php index 2b7ffbd2e..c6fd4c601 100644 --- a/src/Core/BaseKernel.php +++ b/src/Core/BaseKernel.php @@ -114,7 +114,6 @@ abstract class BaseKernel implements Kernel $this->setModuleLoader($moduleLoader); // Config loader - // @todo refactor CoreConfigFactory $configFactory = new CoreConfigFactory($manifestCacheFactory); $configManifest = $configFactory->createRoot(); $configLoader = ConfigLoader::inst(); diff --git a/src/Core/ClassInfo.php b/src/Core/ClassInfo.php index 2e196710a..7c8094d0f 100644 --- a/src/Core/ClassInfo.php +++ b/src/Core/ClassInfo.php @@ -82,8 +82,6 @@ class ClassInfo } /** - * @todo Move this to SS_Database or DB - * * @param string $tableName * @return bool */ @@ -130,8 +128,6 @@ class ClassInfo * Returns an array of the current class and all its ancestors and children * which require a DB table. * - * @todo Move this into {@see DataObjectSchema} - * * @param string|object $nameOrObject Class or object instance * @return array */ diff --git a/src/Core/Convert.php b/src/Core/Convert.php index 161750ae2..0a625bb86 100644 --- a/src/Core/Convert.php +++ b/src/Core/Convert.php @@ -204,7 +204,6 @@ class Convert * Warning: Does not decode array keys * * @uses html2raw() - * @todo Currently &#xxx; entries are stripped; they should be converted * @param mixed $val * @return array|string */ diff --git a/src/Core/Extensible.php b/src/Core/Extensible.php index 3d80cb714..91bf23ce2 100644 --- a/src/Core/Extensible.php +++ b/src/Core/Extensible.php @@ -217,8 +217,6 @@ trait Extensible * Clears any previously created singletons through {@link singleton()} * to avoid side-effects from stale extension information. * - * @todo Add support for removing extensions with parameters - * * @param string $extension class name of an {@link Extension} subclass, without parameters */ public static function remove_extension($extension) diff --git a/src/Core/Injector/Injector.php b/src/Core/Injector/Injector.php index 75b971d0c..75cbf7998 100644 --- a/src/Core/Injector/Injector.php +++ b/src/Core/Injector/Injector.php @@ -666,10 +666,6 @@ class Injector implements ContainerInterface /** * Inject $object with available objects from the service cache * - * @todo Track all the existing objects that have had a service bound - * into them, so we can update that binding at a later point if needbe (ie - * if the managed service changes) - * * @param object $object * The object to inject * @param string $asType diff --git a/src/Dev/BulkLoader.php b/src/Dev/BulkLoader.php index 43961e286..3b9d357d9 100644 --- a/src/Dev/BulkLoader.php +++ b/src/Dev/BulkLoader.php @@ -12,10 +12,6 @@ use SilverStripe\View\ViewableData; * * You can configure column-handling, * - * @todo Add support for adding/editing has_many relations. - * @todo Add support for deep chaining of relation properties (e.g. Player.Team.Stats.GoalCount) - * @todo Character conversion - * * @see http://tools.ietf.org/html/rfc4180 * @author Ingo Schommer, Silverstripe Ltd. (@silverstripe.com) */ @@ -217,7 +213,6 @@ abstract class BulkLoader extends ViewableData * ] * * - * @todo Mix in custom column mappings * * @return array **/ diff --git a/src/Dev/BulkLoader_Result.php b/src/Dev/BulkLoader_Result.php index 333c89a2a..be1b12516 100644 --- a/src/Dev/BulkLoader_Result.php +++ b/src/Dev/BulkLoader_Result.php @@ -11,8 +11,6 @@ use SilverStripe\View\ArrayData; * Encapsulates the result of a {@link BulkLoader} import * (usually through the {@link BulkLoader->processAll()} method). * - * @todo Refactor to support lazy-loaded DataObjectSets once they are implemented. - * * @author Ingo Schommer, Silverstripe Ltd. (@silverstripe.com) */ class BulkLoader_Result implements \Countable diff --git a/src/Dev/CliDebugView.php b/src/Dev/CliDebugView.php index 7347eec48..44121bf78 100644 --- a/src/Dev/CliDebugView.php +++ b/src/Dev/CliDebugView.php @@ -9,8 +9,6 @@ use SilverStripe\Core\Convert; /** * A basic HTML wrapper for stylish rendering of a development info view. * Used to output error messages, and test results. - * - * @todo Perhaps DebugView should be an interface / ABC, implemented by HTMLDebugView and CliDebugView? */ class CliDebugView extends DebugView { diff --git a/src/Dev/Constraint/ViewableDataContains.php b/src/Dev/Constraint/ViewableDataContains.php index af114365b..97bc26e50 100644 --- a/src/Dev/Constraint/ViewableDataContains.php +++ b/src/Dev/Constraint/ViewableDataContains.php @@ -74,8 +74,6 @@ class ViewableDataContains extends Constraint implements TestOnly /** * Returns a string representation of the object. - * - * @todo: add representation for more than one match */ public function toString(): string { diff --git a/src/Dev/CsvBulkLoader.php b/src/Dev/CsvBulkLoader.php index ae8b7bb88..683534449 100644 --- a/src/Dev/CsvBulkLoader.php +++ b/src/Dev/CsvBulkLoader.php @@ -15,9 +15,6 @@ use SilverStripe\ORM\DataObject; * input. * * @see http://tools.ietf.org/html/rfc4180 - * - * @todo Support for deleting existing records not matched in the import - * (through relation checks) */ class CsvBulkLoader extends BulkLoader { @@ -160,9 +157,6 @@ class CsvBulkLoader extends BulkLoader } /** - * @todo Better messages for relation checks and duplicate detection - * Note that columnMap isn't used. - * * @param array $record * @param array $columnMap * @param BulkLoader_Result $results @@ -261,7 +255,6 @@ class CsvBulkLoader extends BulkLoader $obj->write(); } - // @todo better message support $message = ''; // save to results @@ -289,8 +282,6 @@ class CsvBulkLoader extends BulkLoader * Find an existing objects based on one or more uniqueness columns * specified via {@link self::$duplicateChecks}. * - * @todo support $columnMap - * * @param array $record CSV data column * @param array $columnMap * @return DataObject diff --git a/src/Dev/Debug.php b/src/Dev/Debug.php index eb0be06e1..2f759d512 100644 --- a/src/Dev/Debug.php +++ b/src/Dev/Debug.php @@ -25,9 +25,6 @@ use SilverStripe\Security\Security; * Errors handled by this class are passed along to {@link SS_Log}. * For configuration information, see the {@link SS_Log} * class documentation. - * - * @todo add support for user defined config: Debug::die_on_notice(true | false) - * @todo better way of figuring out the error context to display in highlighted source */ class Debug { @@ -212,7 +209,6 @@ class Debug // being called again. // This basically calls Permission::checkMember($_SESSION['loggedInAs'], 'ADMIN'); - // @TODO - Rewrite safely using DataList::filter $memberID = $_SESSION['loggedInAs']; $permission = DB::prepared_query( ' diff --git a/src/Dev/DevelopmentAdmin.php b/src/Dev/DevelopmentAdmin.php index 79b2e1c70..ba135c3c6 100644 --- a/src/Dev/DevelopmentAdmin.php +++ b/src/Dev/DevelopmentAdmin.php @@ -19,11 +19,6 @@ use Exception; * * Configured in framework/_config/dev.yml, with the config key registeredControllers being * used to generate the list of links for /dev. - * - * @todo documentation for how to add new unit tests and tasks - * @todo do we need buildDefaults and generatesecuretoken? if so, register in the list - * @todo cleanup errors() it's not even an allowed action, so can go - * @todo cleanup index() html building */ class DevelopmentAdmin extends Controller { diff --git a/src/Dev/State/ExtensionTestState.php b/src/Dev/State/ExtensionTestState.php index 1c8a62c64..0cf274367 100644 --- a/src/Dev/State/ExtensionTestState.php +++ b/src/Dev/State/ExtensionTestState.php @@ -104,9 +104,6 @@ class ExtensionTestState implements TestState public function tearDownOnce($class) { - // @todo: This isn't strictly necessary to restore extensions, but only to ensure that - // Object::$extra_methods is properly flushed. This should be replaced with a simple - // flush mechanism for each $class. /** @var string|DataObject $dataClass */ // Remove extensions added for testing diff --git a/src/Forms/CheckboxSetField.php b/src/Forms/CheckboxSetField.php index 00925a622..765737e3e 100644 --- a/src/Forms/CheckboxSetField.php +++ b/src/Forms/CheckboxSetField.php @@ -36,11 +36,6 @@ use SilverStripe\View\ArrayData; * the database records. * - If the field name matches a database field, a comma-separated list of values will be saved to that field. The * keys can be text or numbers. - * - * @todo Document the different source data that can be used - * with this form field - e.g ComponentSet, ArrayList, - * array. Is it also appropriate to accept so many different - * types of data when just using an array would be appropriate? */ class CheckboxSetField extends MultiSelectField { @@ -48,9 +43,6 @@ class CheckboxSetField extends MultiSelectField protected $schemaDataType = FormField::SCHEMA_DATA_TYPE_MULTISELECT; /** - * @todo Explain different source data that can be used with this field, - * e.g. SQLMap, ArrayList or an array. - * * @param array $properties * @return DBHTMLText */ diff --git a/src/Forms/CompositeField.php b/src/Forms/CompositeField.php index 0e2c4b35a..d1f9bbf8b 100644 --- a/src/Forms/CompositeField.php +++ b/src/Forms/CompositeField.php @@ -121,8 +121,6 @@ class CompositeField extends FormField * If the CompositeField doesn't have a name, but we still want the ID/name to be set. * This code generates the ID from the nested children. * - * @todo this is temporary, and should be removed when FormTemplateHelper is updated to handle ID for CompositeFields with no name - * * @return String $name */ public function getName() diff --git a/src/Forms/CurrencyField.php b/src/Forms/CurrencyField.php index c340fce7b..5d36cf8ed 100644 --- a/src/Forms/CurrencyField.php +++ b/src/Forms/CurrencyField.php @@ -9,8 +9,6 @@ use SilverStripe\ORM\FieldType\DBCurrency; * Limited to US-centric formats, including a hardcoded currency * symbol and decimal separators. * See {@link MoneyField} for a more flexible implementation. - * - * @todo Add localization support, see http://open.silverstripe.com/ticket/2931 */ class CurrencyField extends TextField { diff --git a/src/Forms/DefaultFormFactory.php b/src/Forms/DefaultFormFactory.php index 669f1cdeb..8b737d048 100644 --- a/src/Forms/DefaultFormFactory.php +++ b/src/Forms/DefaultFormFactory.php @@ -67,7 +67,6 @@ class DefaultFormFactory implements FormFactory protected function getFormFields(RequestHandler $controller = null, $name, $context = []) { // Fall back to standard "getCMSFields" which itself uses the FormScaffolder as a fallback - // @todo Deprecate or formalise support for getCMSFields() $fields = $context['Record']->getCMSFields(); $this->invokeWithExtensions('updateFormFields', $fields, $controller, $name, $context); return $fields; @@ -83,7 +82,6 @@ class DefaultFormFactory implements FormFactory */ protected function getFormActions(RequestHandler $controller = null, $name, $context = []) { - // @todo Deprecate or formalise support for getCMSActions() $actions = $context['Record']->getCMSActions(); $this->invokeWithExtensions('updateFormActions', $actions, $controller, $name, $context); return $actions; diff --git a/src/Forms/FieldGroup.php b/src/Forms/FieldGroup.php index a42b3889a..46099ab1d 100644 --- a/src/Forms/FieldGroup.php +++ b/src/Forms/FieldGroup.php @@ -105,9 +105,6 @@ class FieldGroup extends CompositeField * Returns the name (ID) for the element. * In some cases the FieldGroup doesn't have a title, but we still want * the ID / name to be set. This code, generates the ID from the nested children - * - * TODO this is temporary, and should be removed when FormTemplateHelper is updated to handle ID - * for CompositeFields with no name */ public function getName() { diff --git a/src/Forms/FieldList.php b/src/Forms/FieldList.php index 62156bc36..f84b39135 100644 --- a/src/Forms/FieldList.php +++ b/src/Forms/FieldList.php @@ -411,8 +411,6 @@ class FieldList extends ArrayList /** * Returns the specified tab object, creating it if necessary. * - * @todo Support recursive creation of TabSets - * * @param string $tabName The tab to return, in the form "Tab.Subtab.Subsubtab". * Caution: Does not recursively create TabSet instances, you need to make sure everything * up until the last tab in the chain exists. @@ -462,8 +460,6 @@ class FieldList extends ArrayList * Returns a named field. * You can use dot syntax to get fields from child composite fields * - * @todo Implement similarly to dataFieldByName() to support nested sets - or merge with dataFields() - * * @param string $name * @return FormField|null */ diff --git a/src/Forms/FormField.php b/src/Forms/FormField.php index 763d388b8..dc14e477b 100644 --- a/src/Forms/FormField.php +++ b/src/Forms/FormField.php @@ -123,8 +123,6 @@ class FormField extends RequestHandler * Adds a title attribute to the markup. * * @var string - * - * @todo Implement in all subclasses */ protected $description; @@ -1425,8 +1423,6 @@ class FormField extends RequestHandler * * @param array $schemaData - The data to be merged with $this->schemaData. * @return FormField - * - * @todo Add deep merging of arrays like `data` and `attributes`. */ public function setSchemaData($schemaData = []) { @@ -1447,8 +1443,6 @@ class FormField extends RequestHandler } /** - * @todo Throw exception if value is missing, once a form field schema is mandatory across the CMS - * * @return string */ public function getSchemaDataType() @@ -1501,8 +1495,6 @@ class FormField extends RequestHandler * * @param array $schemaState The data to be merged with $this->schemaData. * @return FormField - * - * @todo Add deep merging of arrays like `data` and `attributes`. */ public function setSchemaState($schemaState = []) { @@ -1529,7 +1521,6 @@ class FormField extends RequestHandler * Includes validation data if the field is associated to a {@link Form}, * and {@link Form->validate()} has been called. * - * @todo Make form / field messages not always stored as html; Store value / casting as separate values. * @return array */ public function getSchemaStateDefaults() diff --git a/src/Forms/FormRequestHandler.php b/src/Forms/FormRequestHandler.php index c52ba236d..1071f92ab 100644 --- a/src/Forms/FormRequestHandler.php +++ b/src/Forms/FormRequestHandler.php @@ -137,7 +137,6 @@ class FormRequestHandler extends RequestHandler $this->form->loadDataFrom($vars, true, $allowedFields); // Protection against CSRF attacks - // @todo Move this to SecurityTokenField::validate() $token = $this->form->getSecurityToken(); if (! $token->checkRequest($request)) { $securityID = $token->getName(); diff --git a/src/Forms/FormScaffolder.php b/src/Forms/FormScaffolder.php index f88d1fb55..d654e73c7 100644 --- a/src/Forms/FormScaffolder.php +++ b/src/Forms/FormScaffolder.php @@ -36,16 +36,12 @@ class FormScaffolder /** * @var array $restrictFields Numeric array of a field name whitelist. * If left blank, all fields from {@link DataObject->db()} will be included. - * - * @todo Implement restrictions for has_many and many_many relations. */ public $restrictFields; /** * @var array $fieldClasses Optional mapping of fieldnames to subclasses of {@link FormField}. * By default the scaffolder will determine the field instance by {@link DBField::scaffoldFormField()}. - * - * @todo Implement fieldClasses for has_many and many_many relations */ public $fieldClasses; @@ -87,7 +83,6 @@ class FormScaffolder continue; } - // @todo Pass localized title if ($this->fieldClasses && isset($this->fieldClasses[$fieldName])) { $fieldClass = $this->fieldClasses[$fieldName]; $fieldObject = new $fieldClass($fieldName); diff --git a/src/Forms/GridField/GridField.php b/src/Forms/GridField/GridField.php index 9b1b3731a..19210651c 100644 --- a/src/Forms/GridField/GridField.php +++ b/src/Forms/GridField/GridField.php @@ -342,8 +342,6 @@ class GridField extends FormField /** * Cast an arbitrary value with the help of a $castingDefinition. * - * @todo refactor this into GridFieldComponent - * * @param mixed $value * @param string|array $castingDefinition * @@ -555,8 +553,6 @@ class GridField extends FormField ]; $fragmentDeferred = []; - // TODO: Break the below into separate reducer methods - // Continue looping if any placeholders exist while (array_filter($content ?? [], function ($value) { return preg_match(self::FRAGMENT_REGEX ?? '', $value ?? ''); @@ -1182,8 +1178,6 @@ class GridField extends FormField * Custom request handler that will check component handlers before proceeding to the default * implementation. * - * @todo copy less code from RequestHandler. - * * @param HTTPRequest $request * @return array|RequestHandler|HTTPResponse|string * @throws HTTPResponse_Exception diff --git a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php index f08ee32b6..a41dd68bb 100644 --- a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php +++ b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php @@ -169,7 +169,6 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler } else { // If not requested by ajax, we need to render it within the controller context+template return $controller->customise([ - // TODO CMS coupling 'Content' => $return, ]); } @@ -179,10 +178,6 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler * Builds an item edit form. The arguments to getCMSFields() are the popupController and * popupFormName, however this is an experimental API and may change. * - * @todo In the future, we will probably need to come up with a tigher object representing a partially - * complete controller with gaps for extra functionality. This, for example, would be a better way - * of letting Security/login put its log-in form inside a UI specified elsewhere. - * * @return Form|HTTPResponse */ public function ItemEditForm() @@ -266,12 +261,10 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler $form->loadDataFrom(['ManyMany' => $extraData]); } - // TODO Coupling with CMS $toplevelController = $this->getToplevelController(); if ($toplevelController && $toplevelController instanceof LeftAndMain) { // Always show with base template (full width, no other panels), // regardless of overloaded CMS controller templates. - // TODO Allow customization, e.g. to display an edit form alongside a search form from the CMS controller $form->setTemplate([ 'type' => 'Includes', 'SilverStripe\\Admin\\LeftAndMain_EditForm', @@ -464,7 +457,6 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler protected function getBackLink() { - // TODO Coupling with CMS $backlink = ''; $toplevelController = $this->getToplevelController(); if ($toplevelController && $toplevelController instanceof LeftAndMain) { diff --git a/src/Forms/GridField/GridFieldExportButton.php b/src/Forms/GridField/GridFieldExportButton.php index e845e4383..d52d9c821 100644 --- a/src/Forms/GridField/GridFieldExportButton.php +++ b/src/Forms/GridField/GridFieldExportButton.php @@ -209,7 +209,6 @@ class GridFieldExportButton extends AbstractGridFieldComponent implements GridFi $items = $gridField->getManipulatedList(); - // @todo should GridFieldComponents change behaviour based on whether others are available in the config? foreach ($gridField->getConfig()->getComponents() as $component) { if ($component instanceof GridFieldFilterHeader || $component instanceof GridFieldSortableHeader) { $items = $component->getManipulatedData($gridField, $items); diff --git a/src/Forms/GroupedDropdownField.php b/src/Forms/GroupedDropdownField.php index f30d8c5c7..59438ff45 100644 --- a/src/Forms/GroupedDropdownField.php +++ b/src/Forms/GroupedDropdownField.php @@ -55,8 +55,6 @@ use SilverStripe\View\ArrayData; class GroupedDropdownField extends DropdownField { - // TODO remove this when GroupedDropdownField is implemented - // This should be one of SCHEMA_DATA_TYPE_* constants instead protected $schemaDataType = 'GroupedDropdownField'; /** diff --git a/src/Forms/MoneyField.php b/src/Forms/MoneyField.php index 6f1c231a8..614d7ebf1 100644 --- a/src/Forms/MoneyField.php +++ b/src/Forms/MoneyField.php @@ -17,7 +17,6 @@ use SilverStripe\ORM\DataObjectInterface; class MoneyField extends FormField { - // TODO replace with `FormField::SCHEMA_DATA_TYPE_TEXT` when MoneyField is implemented protected $schemaDataType = 'MoneyField'; /** diff --git a/src/Forms/SelectionGroup.php b/src/Forms/SelectionGroup.php index 63dd906ae..017905b21 100644 --- a/src/Forms/SelectionGroup.php +++ b/src/Forms/SelectionGroup.php @@ -90,7 +90,6 @@ class SelectionGroup extends CompositeField } $itemID = $this->ID() . '_' . (++$count); - // @todo Move into SelectionGroup_Item.ss template at some point. $extra = [ "RadioButton" => DBField::create_field('HTMLFragment', HTML::createTag( 'input', diff --git a/src/Forms/TimeField.php b/src/Forms/TimeField.php index 525f6dfc4..01d4e99fd 100644 --- a/src/Forms/TimeField.php +++ b/src/Forms/TimeField.php @@ -14,8 +14,6 @@ use SilverStripe\ORM\FieldType\DBTime; * # Localization * * See {@link DateField} - * - * @todo Timezone support */ class TimeField extends TextField { diff --git a/src/Logging/DebugViewFriendlyErrorFormatter.php b/src/Logging/DebugViewFriendlyErrorFormatter.php index 6769c97e4..ae1677133 100644 --- a/src/Logging/DebugViewFriendlyErrorFormatter.php +++ b/src/Logging/DebugViewFriendlyErrorFormatter.php @@ -126,7 +126,6 @@ class DebugViewFriendlyErrorFormatter implements FormatterInterface */ public function output($statusCode) { - // TODO: Refactor into a content-type option if (Director::is_ajax()) { return $this->getTitle(); } diff --git a/src/Logging/HTTPOutputHandler.php b/src/Logging/HTTPOutputHandler.php index 5c2312d0a..b6d922342 100644 --- a/src/Logging/HTTPOutputHandler.php +++ b/src/Logging/HTTPOutputHandler.php @@ -165,8 +165,6 @@ class HTTPOutputHandler extends AbstractProcessingHandler } } - // TODO: This coupling isn't ideal - // See https://github.com/silverstripe/silverstripe-framework/issues/4484 if (Controller::has_curr()) { $response = Controller::curr()->getResponse(); } else { diff --git a/src/ORM/Connect/DBConnector.php b/src/ORM/Connect/DBConnector.php index f192d50b0..5e0413f3f 100644 --- a/src/ORM/Connect/DBConnector.php +++ b/src/ORM/Connect/DBConnector.php @@ -37,7 +37,6 @@ abstract class DBConnector * it will be called on the object itself and as such can be overridden in a subclass. * Subclasses should run all errors through this function. * - * @todo hook this into a more well-structured error handling system. * @param string $msg The error message. * @param integer $errorLevel The level of the error to throw. * @param string $sql The SQL related to this query diff --git a/src/ORM/Connect/DBSchemaManager.php b/src/ORM/Connect/DBSchemaManager.php index 6cc536621..56a50867f 100644 --- a/src/ORM/Connect/DBSchemaManager.php +++ b/src/ORM/Connect/DBSchemaManager.php @@ -350,8 +350,6 @@ abstract class DBSchemaManager * Generate the following table in the database, modifying whatever already exists * as necessary. * - * @todo Change detection for CREATE TABLE $options other than "Engine" - * * @param string $table The name of the table * @param array $fieldSchema A list of the fields to create, in the same form as DataObject::$db * @param array $indexSchema A list of indexes to create. See {@link requireIndex()} @@ -669,7 +667,6 @@ abstract class DBSchemaManager */ public function requireField($table, $field, $spec) { - //TODO: this is starting to get extremely fragmented. //There are two different versions of $spec floating around, and their content changes depending //on how they are structured. This needs to be tidied up. $fieldValue = null; @@ -687,7 +684,6 @@ abstract class DBSchemaManager // Collations didn't come in until MySQL 4.1. Anything earlier will throw a syntax error if you try and use // collations. - // TODO: move this to the MySQLDatabase file, or drop it altogether? if (!$this->database->supportsCollations()) { $spec = preg_replace('/ *character set [^ ]+( collate [^ ]+)?( |$)/', '\\2', $spec ?? ''); } @@ -974,7 +970,6 @@ abstract class DBSchemaManager * @param string $tableName The name of the table. * @param string $indexName The name of the index. * @param array $indexSpec The specification of the index, see Database::requireIndex() for more details. - * @todo Find out where this is called from - Is it even used? Aren't indexes always dropped and re-added? */ abstract public function alterIndex($tableName, $indexName, $indexSpec); diff --git a/src/ORM/Connect/Database.php b/src/ORM/Connect/Database.php index bd2084c8c..2d756f6fd 100644 --- a/src/ORM/Connect/Database.php +++ b/src/ORM/Connect/Database.php @@ -563,7 +563,6 @@ abstract class Database */ public function concatOperator() { - // @todo Make ' + ' in mssql return ' || '; } @@ -714,7 +713,6 @@ abstract class Database * will be an extension name, and the value the configuration for that extension. This * could be one of partitions, tablespaces, or clustering * @return boolean Flag indicating support for all of the above - * @todo Write test cases */ public function supportsExtensions($extensions) { diff --git a/src/ORM/DB.php b/src/ORM/DB.php index c184ddcc3..250a09229 100644 --- a/src/ORM/DB.php +++ b/src/ORM/DB.php @@ -471,8 +471,6 @@ class DB * That's a limitation of the system that's due to it being written for {@link DataObject::write()}, * which needs to do a single write on a number of different tables. * - * @todo Update this to support paramaterised queries - * * @param array $manipulation */ public static function manipulate($manipulation) diff --git a/src/ORM/DataList.php b/src/ORM/DataList.php index 1392933c4..edc3c040c 100644 --- a/src/ORM/DataList.php +++ b/src/ORM/DataList.php @@ -472,8 +472,6 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li * E.g. ->filter('Field:not', 'value) will generate '... OR "Field" IS NULL', and * ->filter('Field:not', null) will generate '"Field" IS NOT NULL' * - * @todo extract the sql from $customQuery into a SQLGenerator class - * * @param string|array Escaped SQL statement. If passed as array, all keys and values will be escaped internally * @return $this */ @@ -536,8 +534,6 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li * $list = $list->filterAny(array('Name'=>array('bob','phil'), 'Age'=>array(21, 43))); * // SQL: WHERE (("Name" IN ('bob', 'phil')) OR ("Age" IN ('21', '43')) * - * @todo extract the sql from this method into a SQLGenerator class - * * @param string|array See {@link filter()} * @return static */ @@ -679,8 +675,6 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li * @example $list = $list->exclude(array('Name'=>array('bob','phil'), 'Age'=>array(21, 43))); * // bob age 21 or 43, phil age 21 or 43 would be excluded * - * @todo extract the sql from this method into a SQLGenerator class - * * @param string|array * @param string [optional] * @@ -1817,7 +1811,6 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li /** * Return a new item to add to this DataList. * - * @todo This doesn't factor in filters. * @param array $initialFields * @return DataObject */ @@ -1831,8 +1824,6 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li * Remove this item by deleting it * * @param DataObject $item - * @todo Allow for amendment of this behaviour - for example, we can remove an item from - * an "ActiveItems" DataList by changing the status to inactive. */ public function remove($item) { diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index b0514fc7b..3e978eb05 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -99,9 +99,6 @@ use stdClass; * the results are cached in memory through {@link cachedCall()}. * * - * @todo Add instance specific removeExtension() which undos loadExtraStatics() - * and defineMethods() - * * @property int $ID ID of the DataObject, 0 if the DataObject doesn't exist in database. * @property int $OldID ID of object, if deleted * @property string $Title @@ -128,8 +125,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity private static $plural_name = null; /** - * Allow API access to this object? - * @todo Define the options that can be set here * @config */ private static $api_access = false; @@ -682,8 +677,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $clonedItem = $item->duplicate(false); $destinationObject->setComponent($relation, $clonedItem); // After $clonedItem is assigned the appropriate FieldID / FieldClass, force write - // @todo Write this component in onAfterWrite instead, assigning the FieldID then - // https://github.com/silverstripe/silverstripe-framework/issues/7818 $clonedItem->write(); } @@ -1151,7 +1144,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity continue; } - // TODO remove redundant merge of has_one fields $leftObj->{$key} = $rightObj->{$key}; } @@ -1752,10 +1744,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity throw new LogicException("DataObject::delete() called on a DataObject without an ID"); } - // TODO: This is quite ugly. To improve: - // - move the details of the delete code in the DataQuery system - // - update the code to just delete the base table, and rely on cascading deletes in the DB to do the rest - // obviously, that means getting requireTable() to configure cascading deletes ;-) $srcQuery = DataList::create(static::class) ->filter('ID', $this->ID) ->dataQuery() @@ -1908,8 +1896,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $schema = static::getSchema(); if ($class = $schema->hasOneComponent(static::class, $componentName)) { // Force item to be written if not by this point - // @todo This could be lazy-written in a beforeWrite hook, but force write here for simplicity - // https://github.com/silverstripe/silverstripe-framework/issues/7818 if ($item && !$item->isInDB()) { $item->write(); } @@ -2594,8 +2580,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * by default. To customize, either overload this method in your * subclass, or extend it by {@link DataExtension->updateFrontEndFields()}. * - * @todo Decide on naming for "website|frontend|site|page" and stick with it in the API - * * @param array $params See {@link scaffoldFormFields()} * @return FieldList Always returns a simple field collection without TabSet. */ @@ -2692,7 +2676,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $columns = []; // Add SQL for fields, both simple & multi-value - // TODO: This is copy & pasted from buildSQL(), it could be moved into a method $databaseFields = $schema->databaseFields($class, false); foreach ($databaseFields as $k => $v) { if (!isset($this->record[$k]) || $this->record[$k] === null) { @@ -2771,7 +2754,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity array_keys($this->record ?? []), array_fill(0, count($this->record ?? []), self::CHANGE_STRICT) ); - // @todo Find better way to allow versioned to write a new version after forceChange unset($changed['Version']); } else { $changed = $this->changed; @@ -2905,8 +2887,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity // if a field is not existing or has strictly changed if (!array_key_exists($fieldName, $this->original ?? []) || $this->original[$fieldName] !== $val) { - // TODO Add check for php-level defaults which are not set in the db - // TODO Add check for hidden input-fields (readonly) which are not set in the db // At the very least, the type has changed $this->changed[$fieldName] = self::CHANGE_STRICT; @@ -3314,8 +3294,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * @param string|array $limit A limit expression to be inserted into the LIMIT clause. * @param string $containerClass The container class to return the results in. * - * @todo $containerClass is Ignored, why? - * * @return DataList The objects matching the filter, in the class specified by $containerClass */ public static function get( @@ -3480,7 +3458,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity */ public static function reset() { - // @todo Decouple these DBEnum::flushCache(); ClassInfo::reset_db_cache(); static::getSchema()->reset(); @@ -3941,8 +3918,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity /** * Get the default summary fields for this object. * - * @todo use the translation apparatus to return a default field selection for the language - * * @return array */ public function summaryFields() @@ -4000,8 +3975,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * it is constructed here. Otherwise, the default filter specified in * {@link DBField} is used. * - * @todo error handling/type checking for valid FormField and SearchFilter subclasses? - * * @return array */ public function defaultSearchFilters() diff --git a/src/ORM/DataObjectSchema.php b/src/ORM/DataObjectSchema.php index dcd7ca17d..09e15149a 100644 --- a/src/ORM/DataObjectSchema.php +++ b/src/ORM/DataObjectSchema.php @@ -1153,7 +1153,7 @@ class DataObjectSchema if ($key === 'from') { return $relationClass; } - // @todo support polymorphic 'to' + throw new InvalidArgumentException( "many_many through relation {$parentClass}.{$component} {$key} references a polymorphic field " . "{$joinClass}::{$relation} which is not supported" diff --git a/src/ORM/DataQuery.php b/src/ORM/DataQuery.php index 2b41c51c4..303117066 100644 --- a/src/ORM/DataQuery.php +++ b/src/ORM/DataQuery.php @@ -63,9 +63,8 @@ class DataQuery */ private $queryFinalised = false; - // TODO: replace subclass_access with this protected $querySubclasses = true; - // TODO: replace restrictclasses with this + protected $filterByClassName = true; /** @@ -335,9 +334,6 @@ class DataQuery "RecordClassName" ); - // TODO: Versioned, SiteTreeSubsites, etc, could probably be better implemented as subclasses - // of DataQuery - $obj = Injector::inst()->get($this->dataClass); $obj->extend('augmentSQL', $query, $this); diff --git a/src/ORM/DatabaseAdmin.php b/src/ORM/DatabaseAdmin.php index 698fe6d3f..c82e3d377 100644 --- a/src/ORM/DatabaseAdmin.php +++ b/src/ORM/DatabaseAdmin.php @@ -519,8 +519,6 @@ class DatabaseAdmin extends Controller /** * Migrate all class names - * - * @todo Migrate to separate build task */ protected function migrateClassNames() { diff --git a/src/ORM/FieldType/DBComposite.php b/src/ORM/FieldType/DBComposite.php index 2447f3df9..c560a6098 100644 --- a/src/ORM/FieldType/DBComposite.php +++ b/src/ORM/FieldType/DBComposite.php @@ -12,8 +12,6 @@ use SilverStripe\ORM\Queries\SQLSelect; * Extend this class when designing a {@link DBField} that doesn't have a 1-1 mapping with a database field. * This includes multi-value fields and transformed fields * - * @todo Unittests for loading and saving composite values (see GIS module for existing similar unittests) - * * Example with a combined street name and number: * * class Street extends DBComposite { diff --git a/src/ORM/FieldType/DBDatetime.php b/src/ORM/FieldType/DBDatetime.php index 8668018a3..ca3c49e7a 100644 --- a/src/ORM/FieldType/DBDatetime.php +++ b/src/ORM/FieldType/DBDatetime.php @@ -30,7 +30,6 @@ use SilverStripe\View\TemplateGlobalProvider; * ); * * - * @todo Add localization support, see http://open.silverstripe.com/ticket/2931 */ class DBDatetime extends DBDate implements TemplateGlobalProvider { diff --git a/src/ORM/FieldType/DBDouble.php b/src/ORM/FieldType/DBDouble.php index ad53e02a9..04b91eab5 100644 --- a/src/ORM/FieldType/DBDouble.php +++ b/src/ORM/FieldType/DBDouble.php @@ -14,7 +14,6 @@ class DBDouble extends DBFloat public function requireField() { // HACK: MSSQL does not support double so we're using float instead - // @todo This should go into MSSQLDatabase ideally somehow if (DB::get_conn() instanceof MySQLDatabase) { DB::require_field($this->tableName, $this->name, "double"); } else { diff --git a/src/ORM/FieldType/DBField.php b/src/ORM/FieldType/DBField.php index 520ce73d8..079df6b8f 100644 --- a/src/ORM/FieldType/DBField.php +++ b/src/ORM/FieldType/DBField.php @@ -41,8 +41,6 @@ use SilverStripe\View\ViewableData; * } * } * - * - * @todo remove MySQL specific code from subclasses */ abstract class DBField extends ViewableData implements DBIndexable { @@ -579,12 +577,6 @@ abstract class DBField extends ViewableData implements DBIndexable } /** - * @todo documentation - * - * @todo figure out how we pass configuration parameters to - * search filters (note: parameter hack now in place to pass in the required full path - using $this->name - * won't work) - * * @param string $name Override name of this field * @return SearchFilter */ diff --git a/src/ORM/FieldType/DBLocale.php b/src/ORM/FieldType/DBLocale.php index fbd947512..9b5e7c614 100644 --- a/src/ORM/FieldType/DBLocale.php +++ b/src/ORM/FieldType/DBLocale.php @@ -6,8 +6,6 @@ use SilverStripe\i18n\i18n; /** * Locale database field - * - * @todo Allowing showing locale values in different languages through Nice() */ class DBLocale extends DBVarchar { diff --git a/src/ORM/FieldType/DBMultiEnum.php b/src/ORM/FieldType/DBMultiEnum.php index 127e54ee0..93cccab8e 100644 --- a/src/ORM/FieldType/DBMultiEnum.php +++ b/src/ORM/FieldType/DBMultiEnum.php @@ -35,7 +35,6 @@ class DBMultiEnum extends DBEnum public function requireField() { - // @todo: Remove mysql-centric logic from this $charset = Config::inst()->get(MySQLDatabase::class, 'charset'); $collation = Config::inst()->get(MySQLDatabase::class, 'collation'); $values = [ diff --git a/src/ORM/FieldType/DBPolymorphicForeignKey.php b/src/ORM/FieldType/DBPolymorphicForeignKey.php index a2c680c5c..a70be359c 100644 --- a/src/ORM/FieldType/DBPolymorphicForeignKey.php +++ b/src/ORM/FieldType/DBPolymorphicForeignKey.php @@ -18,9 +18,8 @@ class DBPolymorphicForeignKey extends DBComposite public function scaffoldFormField($title = null, $params = null) { - // Opt-out of form field generation - Scaffolding should be performed on + // Don't provide scaffolded form field generation - Scaffolding should be performed on // the has_many end, or set programmatically. - // @todo - Investigate suitable FormField return null; } diff --git a/src/ORM/FieldType/DBPrimaryKey.php b/src/ORM/FieldType/DBPrimaryKey.php index 8132bfa3f..7e9a207e2 100644 --- a/src/ORM/FieldType/DBPrimaryKey.php +++ b/src/ORM/FieldType/DBPrimaryKey.php @@ -7,8 +7,6 @@ use SilverStripe\ORM\DB; /** * A special type Int field used for primary keys. - * - * @todo Allow for custom limiting/filtering of scaffoldFormField dropdown */ class DBPrimaryKey extends DBInt { diff --git a/src/ORM/Filters/ExactMatchFilter.php b/src/ORM/Filters/ExactMatchFilter.php index 5f4d1dfc3..077018434 100644 --- a/src/ORM/Filters/ExactMatchFilter.php +++ b/src/ORM/Filters/ExactMatchFilter.php @@ -13,7 +13,6 @@ use SilverStripe\ORM\DataList; /** * Selects textual content with an exact match between columnname and keyword. * - * @todo documentation */ class ExactMatchFilter extends SearchFilter { diff --git a/src/ORM/Filters/FulltextFilter.php b/src/ORM/Filters/FulltextFilter.php index d49b559dd..d0de1f8ca 100755 --- a/src/ORM/Filters/FulltextFilter.php +++ b/src/ORM/Filters/FulltextFilter.php @@ -25,7 +25,6 @@ use Exception; * ]; * * - * @todo Add support for databases besides MySQL */ class FulltextFilter extends SearchFilter { diff --git a/src/ORM/Filters/SearchFilter.php b/src/ORM/Filters/SearchFilter.php index 204a11159..daf06f987 100644 --- a/src/ORM/Filters/SearchFilter.php +++ b/src/ORM/Filters/SearchFilter.php @@ -332,7 +332,6 @@ abstract class SearchFilter public function getDbFormattedValue() { // SRM: This code finds the table where the field named $this->name lives - // Todo: move to somewhere more appropriate, such as DataMapper, the magical class-to-be? if ($this->aggregate) { return intval($this->value); diff --git a/src/ORM/Filters/WithinRangeFilter.php b/src/ORM/Filters/WithinRangeFilter.php index 8d3da4592..55f943e33 100644 --- a/src/ORM/Filters/WithinRangeFilter.php +++ b/src/ORM/Filters/WithinRangeFilter.php @@ -4,11 +4,6 @@ namespace SilverStripe\ORM\Filters; use SilverStripe\ORM\DataQuery; -/** - * Incomplete. - * - * @todo add to tests - */ class WithinRangeFilter extends SearchFilter { diff --git a/src/ORM/HasManyList.php b/src/ORM/HasManyList.php index 2837ccdf6..a28abfadb 100644 --- a/src/ORM/HasManyList.php +++ b/src/ORM/HasManyList.php @@ -119,7 +119,6 @@ class HasManyList extends RelationList * Doesn't actually remove the item, it just clears the foreign key value. * * @param DataObject $item The DataObject to be removed - * @todo Maybe we should delete the object instead? */ public function remove($item) { diff --git a/src/ORM/PolymorphicHasManyList.php b/src/ORM/PolymorphicHasManyList.php index 20f9907c1..499d8e003 100644 --- a/src/ORM/PolymorphicHasManyList.php +++ b/src/ORM/PolymorphicHasManyList.php @@ -111,7 +111,6 @@ class PolymorphicHasManyList extends HasManyList * Doesn't actually remove the item, it just clears the foreign key value. * * @param DataObject $item The DataObject to be removed - * @todo Maybe we should delete the object instead? */ public function remove($item) { diff --git a/src/ORM/Queries/SQLAssignmentRow.php b/src/ORM/Queries/SQLAssignmentRow.php index bf4bd9c5b..17a1a51d5 100644 --- a/src/ORM/Queries/SQLAssignmentRow.php +++ b/src/ORM/Queries/SQLAssignmentRow.php @@ -68,8 +68,6 @@ class SQLAssignmentRow $parameters = [$parameters]; } - // @todo Some input sanitisation checking the key contains the - // correct number of ? placeholders as the number of parameters return [$sql => $parameters]; } } diff --git a/src/ORM/Queries/SQLConditionalExpression.php b/src/ORM/Queries/SQLConditionalExpression.php index 55d6ae5c2..55aef214a 100644 --- a/src/ORM/Queries/SQLConditionalExpression.php +++ b/src/ORM/Queries/SQLConditionalExpression.php @@ -256,8 +256,6 @@ abstract class SQLConditionalExpression extends SQLExpression /** * Retrieves the finalized list of joins * - * @todo This part of the code could be simplified - * * @param array $parameters Out variable for parameters required for this query * @return array List of joins as a mapping from array('Alias' => 'Join Expression') */ @@ -693,8 +691,6 @@ abstract class SQLConditionalExpression extends SQLExpression /** * Checks whether this query is for a specific ID in a table * - * @todo Doesn't work with combined statements (e.g. "Foo='bar' AND ID=5") - * * @return boolean */ public function filtersOnID() @@ -713,15 +709,12 @@ abstract class SQLConditionalExpression extends SQLExpression /** * Checks whether this query is filtering on a foreign key, ie finding a has_many relationship * - * @todo Doesn't work with combined statements (e.g. "Foo='bar' AND ParentID=5") - * * @return boolean */ public function filtersOnFK() { $regexp = '/^(.*\.)?("|`)?[a-zA-Z]+ID("|`)?\s?(=|IN)/'; - // @todo - Test this works with parameterized queries foreach ($this->getWhereParameterised($parameters) as $predicate) { if (preg_match($regexp ?? '', $predicate ?? '')) { return true; diff --git a/src/ORM/Queries/SQLExpression.php b/src/ORM/Queries/SQLExpression.php index f7e261726..168f943d8 100644 --- a/src/ORM/Queries/SQLExpression.php +++ b/src/ORM/Queries/SQLExpression.php @@ -45,8 +45,6 @@ abstract class SQLExpression /** * Return the generated SQL string for this query * - * @todo Is it ok for this to consider parameters? Test cases here! - * * @return string */ public function __toString() diff --git a/src/ORM/Queries/SQLSelect.php b/src/ORM/Queries/SQLSelect.php index fa38005aa..c016b77ef 100644 --- a/src/ORM/Queries/SQLSelect.php +++ b/src/ORM/Queries/SQLSelect.php @@ -575,7 +575,6 @@ class SQLSelect extends SQLConditionalExpression // Choose a default column if ($column == null) { if ($this->groupby) { - // @todo Test case required here $countQuery = new SQLSelect(); $countQuery->setSelect("count(*)"); $countQuery->setFrom(['(' . $clone->sql($innerParameters) . ') all_distinct']); diff --git a/src/ORM/Search/SearchContext.php b/src/ORM/Search/SearchContext.php index 535af6bcb..978ba3677 100644 --- a/src/ORM/Search/SearchContext.php +++ b/src/ORM/Search/SearchContext.php @@ -107,10 +107,6 @@ class SearchContext return ($this->fields) ? $this->fields : singleton($this->modelClass)->scaffoldSearchFields(); } - /** - * @todo move to SQLSelect - * @todo fix hack - */ protected function applyBaseTableFields() { $classes = ClassInfo::dataClassesFor($this->modelClass); @@ -327,8 +323,6 @@ class SearchContext /** * Returns a result set from the given search parameters. * - * @todo rearrange start and limit params to reflect DataObject - * * @param array $searchParams * @param array|bool|string $sort * @param array|null|string $limit diff --git a/src/Security/Group.php b/src/Security/Group.php index 502a1fea3..7bf3ed7c9 100755 --- a/src/Security/Group.php +++ b/src/Security/Group.php @@ -183,8 +183,6 @@ class Group extends DataObject if ($record && !$record->ID) { $groupsField->setValue($group->ID); } elseif ($record && $record->ID) { - // TODO Mark disabled once chosen.js supports it - // $groupsField->setDisabledItems(array($group->ID)); $form->Fields()->replaceField( 'DirectGroups', $groupsField->performReadonlyTransformation() @@ -194,8 +192,6 @@ class Group extends DataObject }); $memberList = GridField::create('Members', false, $this->DirectMembers(), $config) ->addExtraClass('members_grid'); - // @todo Implement permission checking on GridField - //$memberList->setPermissions(array('edit', 'delete', 'export', 'add', 'inlineadd')); $fields->addFieldToTab('Root.Members', $memberList); } @@ -238,8 +234,6 @@ class Group extends DataObject sprintf( '%s', SecurityAdmin::singleton()->Link('show/root#Root_Roles'), - // TODO This should include #Root_Roles to switch directly to the tab, - // but tabstrip.js doesn't display tabs when directly addressed through a URL pragma _t('SilverStripe\\Security\\Group.RolesAddEditLink', 'Manage roles') ) . "

" diff --git a/src/Security/GroupCsvBulkLoader.php b/src/Security/GroupCsvBulkLoader.php index 40db7d469..64bc8079c 100644 --- a/src/Security/GroupCsvBulkLoader.php +++ b/src/Security/GroupCsvBulkLoader.php @@ -5,9 +5,6 @@ namespace SilverStripe\Security; use SilverStripe\Dev\CsvBulkLoader; use SilverStripe\ORM\DataObject; -/** - * @todo Migrate Permission->Arg and Permission->Type values - */ class GroupCsvBulkLoader extends CsvBulkLoader { diff --git a/src/Security/Member.php b/src/Security/Member.php index 2dffce485..dc309b0da 100644 --- a/src/Security/Member.php +++ b/src/Security/Member.php @@ -114,9 +114,6 @@ class Member extends DataObject * statement. * * @var array - * @todo Generic implementation of $searchable_fields on DataObject, - * with definition for different searching algorithms - * (LIKE, FULLTEXT) and default FormFields to construct a searchform. */ private static $searchable_fields = [ 'FirstName', @@ -446,7 +443,6 @@ class Member extends DataObject */ public function beforeMemberLoggedIn() { - // @todo Move to middleware on the AuthenticationMiddleware IdentityStore $this->extend('beforeMemberLoggedIn'); } @@ -619,7 +615,6 @@ class Member extends DataObject * Returns the fields for the member form - used in the registration/profile module. * It should return fields that are editable by the admin and the logged-in user. * - * @todo possibly move this to an extension * * @return FieldList Returns a {@link FieldList} containing the fields for * the member form. @@ -770,7 +765,6 @@ class Member extends DataObject // We don't send emails out on dev/tests sites to prevent accidentally spamming users. // However, if TestMailer is in use this isn't a risk. - // @todo some developers use external tools, so emailing might be a good idea anyway if ((Director::isLive() || Injector::inst()->get(MailerInterface::class) instanceof TestMailer) && $this->isChanged('Password') && $this->record['Password'] @@ -1151,7 +1145,6 @@ class Member extends DataObject * including any parent groups where membership is implied. * Use {@link DirectGroups()} to only retrieve the group relations without inheritance. * - * @todo Push all this logic into Member_GroupSet's getIterator()? * @return Member_Groupset */ public function Groups() diff --git a/src/Security/MemberAuthenticator/ChangePasswordHandler.php b/src/Security/MemberAuthenticator/ChangePasswordHandler.php index 43cc600fa..b2330f5b2 100644 --- a/src/Security/MemberAuthenticator/ChangePasswordHandler.php +++ b/src/Security/MemberAuthenticator/ChangePasswordHandler.php @@ -58,7 +58,6 @@ class ChangePasswordHandler extends RequestHandler /** * Handle the change password request - * @todo this could use some spring cleaning * * @return array|HTTPResponse */ diff --git a/src/Security/MemberAuthenticator/CookieAuthenticationHandler.php b/src/Security/MemberAuthenticator/CookieAuthenticationHandler.php index 54f710de6..677de4710 100644 --- a/src/Security/MemberAuthenticator/CookieAuthenticationHandler.php +++ b/src/Security/MemberAuthenticator/CookieAuthenticationHandler.php @@ -173,13 +173,9 @@ class CookieAuthenticationHandler implements AuthenticationHandler } if ($this->cascadeInTo) { - // @todo look at how to block "regular login" triggers from happening here - // @todo deal with the fact that the Session::current_session() isn't correct here :-/ $this->cascadeInTo->logIn($member, false, $request); } - // @todo Consider whether response should be part of logIn() as well - // Renew the token $rememberLoginHash->renew(); $tokenExpiryDays = RememberLoginHash::config()->uninherited('token_expiry_days'); diff --git a/src/Security/MemberAuthenticator/MemberAuthenticator.php b/src/Security/MemberAuthenticator/MemberAuthenticator.php index 755d83c8d..6c2d83ecb 100644 --- a/src/Security/MemberAuthenticator/MemberAuthenticator.php +++ b/src/Security/MemberAuthenticator/MemberAuthenticator.php @@ -168,7 +168,6 @@ class MemberAuthenticator implements Authenticator /** * Log login attempt - * TODO We could handle this with an extension * * @param array $data * @param HTTPRequest $request diff --git a/src/Security/MemberAuthenticator/MemberLoginForm.php b/src/Security/MemberAuthenticator/MemberLoginForm.php index 1a31e55a2..94dd5027d 100644 --- a/src/Security/MemberAuthenticator/MemberLoginForm.php +++ b/src/Security/MemberAuthenticator/MemberLoginForm.php @@ -84,7 +84,6 @@ class MemberLoginForm extends BaseLoginForm } if ($checkCurrentUser && Security::getCurrentUser()) { - // @todo find a more elegant way to handle this $logoutAction = Security::logout_url(); $fields = FieldList::create( HiddenField::create('AuthenticationMethod', null, $this->getAuthenticatorClass(), $this) @@ -134,7 +133,6 @@ class MemberLoginForm extends BaseLoginForm HiddenField::create("AuthenticationMethod", null, $this->getAuthenticatorClass(), $this), // Regardless of what the unique identifier field is (usually 'Email'), it will be held in the // 'Email' value, below: - // @todo Rename the field to a more generic covering name $emailField = TextField::create("Email", $label, null, null, $this), PasswordField::create("Password", _t('SilverStripe\\Security\\Member.PASSWORD', 'Password')) ); diff --git a/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php b/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php index 8036f6039..5dec89702 100644 --- a/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php +++ b/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php @@ -80,7 +80,6 @@ class SessionAuthenticationHandler implements AuthenticationHandler $session->set($this->getSessionVariable(), $member->ID); // This lets apache rules detect whether the user has logged in - // @todo make this a setting on the authentication handler if (Member::config()->get('login_marker_cookie')) { Cookie::set(Member::config()->get('login_marker_cookie'), 1, 0); } @@ -109,7 +108,6 @@ class SessionAuthenticationHandler implements AuthenticationHandler $file = ''; $line = ''; - // TODO: deprecate and use Session::regenerateSessionId // @ is to suppress win32 warnings/notices when session wasn't cleaned up properly // There's nothing we can do about this, because it's an operating system function! if (!headers_sent($file, $line)) { diff --git a/src/Security/MemberCsvBulkLoader.php b/src/Security/MemberCsvBulkLoader.php index 35c255d6a..cde99eb97 100644 --- a/src/Security/MemberCsvBulkLoader.php +++ b/src/Security/MemberCsvBulkLoader.php @@ -45,7 +45,6 @@ class MemberCsvBulkLoader extends CsvBulkLoader /** @var Member $member */ $member = DataObject::get_by_id($this->objectClass, $objID); foreach ($this->groups as $group) { - // TODO This isnt the most memory effective way to add members to a group $member->Groups()->add($group); } diff --git a/src/Security/Permission.php b/src/Security/Permission.php index 8e1dad98f..c15757f57 100644 --- a/src/Security/Permission.php +++ b/src/Security/Permission.php @@ -133,7 +133,6 @@ class Permission extends DataObject implements TemplateGlobalProvider, Resettabl /** * Flush the permission cache, for example if you have edited group membership or a permission record. - * @todo Call this whenever Group_Members is added to or removed from */ public static function reset() { diff --git a/src/Security/Security.php b/src/Security/Security.php index 215059772..82bfb5bc8 100644 --- a/src/Security/Security.php +++ b/src/Security/Security.php @@ -416,7 +416,6 @@ class Security extends Controller implements TemplateGlobalProvider $request->getSession()->set("BackURL", $_SERVER['REQUEST_URI']); } - // TODO AccessLogEntry needs an extension to handle permission denied errors // Audit logging hook $controller->extend('permissionDenied', $member); diff --git a/src/Security/SecurityToken.php b/src/Security/SecurityToken.php index f54eb6c5d..e6ba9c2f0 100644 --- a/src/Security/SecurityToken.php +++ b/src/Security/SecurityToken.php @@ -35,7 +35,6 @@ use SilverStripe\View\TemplateGlobalProvider; * } * * - * @todo Make token name form specific for additional forgery protection. */ class SecurityToken implements TemplateGlobalProvider { diff --git a/src/View/SSTemplateParser.php b/src/View/SSTemplateParser.php index d1778c49b..db9f0a6f3 100644 --- a/src/View/SSTemplateParser.php +++ b/src/View/SSTemplateParser.php @@ -1886,8 +1886,6 @@ class SSTemplateParser extends Parser implements TemplateParser $res['php'] .= '((bool)'.$sub['php'].')'; } else { $php = ($sub['ArgumentMode'] == 'default' ? $sub['lookup_php'] : $sub['php']); - // TODO: kinda hacky - maybe we need a way to pass state down the parse chain so - // Lookup_LastLookupStep and Argument_BareWord can produce hasValue instead of XML_val $res['php'] .= str_replace('$$FINAL', 'hasValue', $php ?? ''); } } @@ -5292,8 +5290,6 @@ class SSTemplateParser extends Parser implements TemplateParser $text = stripslashes($text ?? ''); $text = addcslashes($text ?? '', '\'\\'); - // TODO: This is pretty ugly & gets applied on all files not just html. I wonder if we can make this - // non-dynamically calculated $code = <<<'EOC' (\SilverStripe\View\SSViewer::getRewriteHashLinksDefault() ? \SilverStripe\Core\Convert::raw2att( preg_replace("/^(\\/)+/", "/", $_SERVER['REQUEST_URI'] ) ) @@ -5332,8 +5328,7 @@ EOC; $this->includeDebuggingComments = $includeDebuggingComments; - // Ignore UTF8 BOM at beginning of string. TODO: Confirm this is needed, make sure SSViewer handles UTF - // (and other encodings) properly + // Ignore UTF8 BOM at beginning of string. if (substr($string ?? '', 0, 3) == pack("CCC", 0xef, 0xbb, 0xbf)) { $this->pos = 3; } diff --git a/src/View/ViewableData.php b/src/View/ViewableData.php index 7c3d3f814..31f621b5e 100644 --- a/src/View/ViewableData.php +++ b/src/View/ViewableData.php @@ -438,8 +438,6 @@ class ViewableData implements IteratorAggregate { $class = $this->castingClass($field) ?: $this->config()->get('default_cast'); - // TODO: It would be quicker not to instantiate the object, but to merely - // get its class from the Injector /** @var DBField $type */ $type = Injector::inst()->get($class, true); return $type->config()->get('escape_type'); diff --git a/src/i18n/TextCollection/i18nTextCollector.php b/src/i18n/TextCollection/i18nTextCollector.php index 145f93b0f..fce88716d 100644 --- a/src/i18n/TextCollection/i18nTextCollector.php +++ b/src/i18n/TextCollection/i18nTextCollector.php @@ -71,8 +71,6 @@ class i18nTextCollector * The directory base on which the collector should act. * Usually the webroot set through {@link Director::baseFolder()}. * - * @todo Fully support changing of basePath through {@link SSViewer} and {@link ManifestBuilder} - * * @var string */ public $basePath; @@ -314,9 +312,6 @@ class i18nTextCollector return $owner; } - // @todo - How to determine ownership of templates? Templates can - // exist in multiple locations with the same name. - // Display notice if not found Debug::message( "Duplicate key {$key} detected in no / multiple modules with no obvious owner", diff --git a/tests/behat/src/CmsFormsContext.php b/tests/behat/src/CmsFormsContext.php index df9ae575d..3c471fcbf 100644 --- a/tests/behat/src/CmsFormsContext.php +++ b/tests/behat/src/CmsFormsContext.php @@ -137,7 +137,6 @@ class CmsFormsContext implements Context * Example: Given "my text" in the "Content" HTML field should be right aligned * Example: Given "my text" in the "Content" HTML field should not be bold * - * @todo Use an actual DOM parser for more accurate assertions * * @Given /^"(?P([^"]*))" in the "(?P(?:[^"]|\\")*)" HTML field should(?P(?: not)?) be (?P(.*))$/ */ @@ -194,8 +193,7 @@ class CmsFormsContext implements Context $text = addcslashes($text ?? '', "'"); $js = <<getSession()->switchToIFrame('cms-preview-iframe'); $link = $this->fixStepArgument($link); $this->getSession()->getPage()->clickLink($link); diff --git a/tests/php/Control/DirectorTest.php b/tests/php/Control/DirectorTest.php index a9e417ec9..f0332322e 100644 --- a/tests/php/Control/DirectorTest.php +++ b/tests/php/Control/DirectorTest.php @@ -18,9 +18,6 @@ use SilverStripe\Core\Environment; use SilverStripe\Core\Kernel; use SilverStripe\Dev\SapphireTest; -/** - * @todo test Director::alternateBaseFolder() - */ class DirectorTest extends SapphireTest { protected static $extra_controllers = [ diff --git a/tests/php/Control/SessionTest.php b/tests/php/Control/SessionTest.php index e0b4d4c49..3cae00714 100644 --- a/tests/php/Control/SessionTest.php +++ b/tests/php/Control/SessionTest.php @@ -133,9 +133,6 @@ class SessionTest extends SapphireTest public function testStartRetainsInMemoryData() { $this->markTestIncomplete('Test'); - // TODO Figure out how to simulate session vars without a session_start() resetting them - // $_SESSION['existing'] = true; - // $_SESSION['merge'] = 1; $req = new HTTPRequest('GET', '/'); $session = new Session(null); // unstarted session $session->set('new', true); diff --git a/tests/php/Core/ConvertTest.php b/tests/php/Core/ConvertTest.php index 243694bee..a4d0d73d0 100644 --- a/tests/php/Core/ConvertTest.php +++ b/tests/php/Core/ConvertTest.php @@ -214,8 +214,6 @@ PHP /** * Tests {@link Convert::testRaw2URL()} - * - * @todo test toASCII() */ public function testRaw2URL() { diff --git a/tests/php/Core/Manifest/ThemeResourceLoaderTest.php b/tests/php/Core/Manifest/ThemeResourceLoaderTest.php index e02761b50..0c4c424f7 100644 --- a/tests/php/Core/Manifest/ThemeResourceLoaderTest.php +++ b/tests/php/Core/Manifest/ThemeResourceLoaderTest.php @@ -213,7 +213,6 @@ class ThemeResourceLoaderTest extends SapphireTest */ public function testFindTemplatesInApplication() { - // TODO: replace with one that doesn't create temporary files (so bad) $templates = [ $this->base . '/myproject/templates/Page.ss', $this->base . '/myproject/templates/Layout/Page.ss' diff --git a/tests/php/Core/ObjectTest.php b/tests/php/Core/ObjectTest.php index bf1107586..32dfaa2fd 100644 --- a/tests/php/Core/ObjectTest.php +++ b/tests/php/Core/ObjectTest.php @@ -22,11 +22,6 @@ use SilverStripe\Core\Tests\ObjectTest\TestExtension; use SilverStripe\Dev\SapphireTest; use SilverStripe\Versioned\Versioned; -/** - * @todo tests for addStaticVars() - * @todo tests for setting statics which are not defined on the object as built-in PHP statics - * @todo tests for setting statics through extensions (#2387) - */ class ObjectTest extends SapphireTest { @@ -253,8 +248,6 @@ class ObjectTest extends SapphireTest "Injected sub-extensions are detected with static has_extension() when added through add_extension()" ); - // @todo At the moment, this does NOT remove the extension due to parameterized naming, - // meaning the extension will remain added in further test cases ExtensionTest::remove_extension(ExtendTest3::class); } diff --git a/tests/php/Core/PhpSyntaxTest.php b/tests/php/Core/PhpSyntaxTest.php index 77ce2c942..53134479d 100644 --- a/tests/php/Core/PhpSyntaxTest.php +++ b/tests/php/Core/PhpSyntaxTest.php @@ -18,7 +18,6 @@ class PhpSyntaxTest extends SapphireTest public function testShortTagsOffWillWork() { // Ignore this test completely if running the test suite on windows - // TODO: Make it work on all platforms, by building an alternative to find | grep. $returnCode = 0; $output = []; exec("which find && which grep && which php", $output, $returnCode); @@ -51,7 +50,6 @@ class PhpSyntaxTest extends SapphireTest public function getAllFiles($ext = 'php') { - // TODO: Unix only $cmd = sprintf( 'find %s | grep %s', BASE_PATH, diff --git a/tests/php/Dev/CsvBulkLoaderTest.php b/tests/php/Dev/CsvBulkLoaderTest.php index c6c65e5e0..93388fbf4 100644 --- a/tests/php/Dev/CsvBulkLoaderTest.php +++ b/tests/php/Dev/CsvBulkLoaderTest.php @@ -227,8 +227,6 @@ class CsvBulkLoaderTest extends SapphireTest /** * Test import with custom identifiers by importing the data. - * - * @todo Test duplicateCheck callbacks */ public function testLoadWithIdentifiers() { diff --git a/tests/php/Forms/CurrencyFieldDisabledTest.php b/tests/php/Forms/CurrencyFieldDisabledTest.php index d02e1375a..ed9c8c5ac 100644 --- a/tests/php/Forms/CurrencyFieldDisabledTest.php +++ b/tests/php/Forms/CurrencyFieldDisabledTest.php @@ -18,9 +18,6 @@ class CurrencyFieldDisabledTest extends SapphireTest $this->assertStringContainsString('$5.00', $result, 'The value should be rendered'); } - /** - * @todo: Update the expectation when intl for currencies is implemented - */ public function testFieldWithCustomisedCurrencySymbol() { DBCurrency::config()->set('currency_symbol', '€'); diff --git a/tests/php/Forms/CurrencyFieldReadonlyTest.php b/tests/php/Forms/CurrencyFieldReadonlyTest.php index 352b97f82..7850a79f5 100644 --- a/tests/php/Forms/CurrencyFieldReadonlyTest.php +++ b/tests/php/Forms/CurrencyFieldReadonlyTest.php @@ -37,9 +37,6 @@ class CurrencyFieldReadonlyTest extends SapphireTest $this->assertStringContainsString('AUD0.00', $result, 'The value should be rendered'); } - /** - * @todo: Update the expectation when intl for currencies is implemented - */ public function testFieldWithCustomisedCurrencySymbol() { DBCurrency::config()->set('currency_symbol', '€'); diff --git a/tests/php/Forms/EmailFieldTest.php b/tests/php/Forms/EmailFieldTest.php index 3ec5a7acd..26723e555 100644 --- a/tests/php/Forms/EmailFieldTest.php +++ b/tests/php/Forms/EmailFieldTest.php @@ -18,8 +18,6 @@ class EmailFieldTest extends FunctionalTest /** * Check the php validator for email addresses. We should be checking against RFC 5322 which defines email address * syntax. - * - * @TODO * - double quotes around the local part (before @) is not supported * - special chars ! # $ % & ' * + - / = ? ^ _ ` { | } ~ are all valid in local part * - special chars ()[]\;:,<> are valid in the local part if the local part is in double quotes diff --git a/tests/php/Forms/FieldListTest.php b/tests/php/Forms/FieldListTest.php index 2718aaaf0..ec235dfee 100644 --- a/tests/php/Forms/FieldListTest.php +++ b/tests/php/Forms/FieldListTest.php @@ -20,17 +20,6 @@ use SilverStripe\Forms\HiddenField; /** * Tests for FieldList - * - * @todo test for {@link FieldList->setValues()}. Need to check - * that the values that were set are the correct ones given back. - * @todo test for {@link FieldList->transform()} and {@link FieldList->makeReadonly()}. - * Need to ensure that it correctly transforms the FieldList object. - * @todo test for {@link FieldList->HiddenFields()}. Need to check - * the fields returned are the correct HiddenField objects for a - * given FieldList instance. - * @todo test for {@link FieldList->dataFields()}. - * @todo test for {@link FieldList->findOrMakeTab()}. - * @todo the same as above with insertBefore() and insertAfter() */ class FieldListTest extends SapphireTest { @@ -856,9 +845,6 @@ class FieldListTest extends SapphireTest ); } - /** - * @todo check actual placement of fields - */ public function testInsertBeforeWithNestedTabsets() { $FieldListA = new FieldList( @@ -973,9 +959,6 @@ class FieldListTest extends SapphireTest ); } - /** - * @todo check actual placement of fields - */ public function testInsertAfterWithNestedTabsets() { $FieldListA = new FieldList( diff --git a/tests/php/Forms/GridField/GridFieldDetailFormTest/Category.php b/tests/php/Forms/GridField/GridFieldDetailFormTest/Category.php index c4a7c9f01..77249e0cd 100644 --- a/tests/php/Forms/GridField/GridFieldDetailFormTest/Category.php +++ b/tests/php/Forms/GridField/GridFieldDetailFormTest/Category.php @@ -24,7 +24,6 @@ class Category extends DataObject implements TestOnly public function getCMSFields() { $fields = parent::getCMSFields(); - // TODO No longer necessary once FormScaffolder uses GridField $fields->replaceField( 'People', GridField::create( diff --git a/tests/php/Forms/GridField/GridFieldDetailFormTest/PeopleGroup.php b/tests/php/Forms/GridField/GridFieldDetailFormTest/PeopleGroup.php index f68da4918..9530132e0 100644 --- a/tests/php/Forms/GridField/GridFieldDetailFormTest/PeopleGroup.php +++ b/tests/php/Forms/GridField/GridFieldDetailFormTest/PeopleGroup.php @@ -29,7 +29,6 @@ class PeopleGroup extends DataObject implements TestOnly public function getCMSFields() { $fields = parent::getCMSFields(); - // TODO No longer necessary once FormScaffolder uses GridField $fields->replaceField( 'People', GridField::create( diff --git a/tests/php/Forms/GridField/GridFieldDetailFormTest/Person.php b/tests/php/Forms/GridField/GridFieldDetailFormTest/Person.php index 9b76b3d2c..2d4ac82c0 100644 --- a/tests/php/Forms/GridField/GridFieldDetailFormTest/Person.php +++ b/tests/php/Forms/GridField/GridFieldDetailFormTest/Person.php @@ -39,7 +39,6 @@ class Person extends DataObject implements TestOnly public function getCMSFields() { $fields = parent::getCMSFields(); - // TODO No longer necessary once FormScaffolder uses GridField $fields->replaceField( 'Categories', GridField::create( diff --git a/tests/php/Forms/GridField/GridFieldTest.php b/tests/php/Forms/GridField/GridFieldTest.php index 1b33db759..642460aa5 100644 --- a/tests/php/Forms/GridField/GridFieldTest.php +++ b/tests/php/Forms/GridField/GridFieldTest.php @@ -185,10 +185,6 @@ class GridFieldTest extends SapphireTest { $obj = new GridField('testfield', 'testfield'); - // @todo - PHP 7.0.6 change requires __isset() to return true - // for each reference from left to right along an isset() invocation. - // See https://bugs.php.net/bug.php?id=62059 - // Check value persistence $this->assertEquals(15, $obj->State->NoValue(15)); $this->assertEquals(15, $obj->State->NoValue(-1)); diff --git a/tests/php/Forms/ListboxFieldTest.php b/tests/php/Forms/ListboxFieldTest.php index 8591164e7..085b0758f 100644 --- a/tests/php/Forms/ListboxFieldTest.php +++ b/tests/php/Forms/ListboxFieldTest.php @@ -222,15 +222,6 @@ class ListboxFieldTest extends SapphireTest 'Field validates values in source map' ); - /** - * @todo re-enable these tests when field validation is removed from {@link ListboxField::setValue()} and moved - * to the {@link ListboxField::validate()} function - */ - // $field->setValue(4); - // $this->assertFalse( - // $field->validate($validator), - // 'Field does not validate values outside of source map' - // ); $field->setValue( false, new ArrayData( diff --git a/tests/php/Forms/NumericFieldTest.php b/tests/php/Forms/NumericFieldTest.php index f07a6dfc5..567d9a436 100644 --- a/tests/php/Forms/NumericFieldTest.php +++ b/tests/php/Forms/NumericFieldTest.php @@ -128,9 +128,6 @@ class NumericFieldTest extends SapphireTest $field = new NumericField('Number'); $html = $field->Field(); - - // @todo - Revert to number one day when html5 number supports proper localisation - // See https://github.com/silverstripe/silverstripe-framework/pull/4565 $this->assertStringContainsString('type="text"', $html, 'number type not set'); } diff --git a/tests/php/Forms/RequiredFieldsTest.php b/tests/php/Forms/RequiredFieldsTest.php index cb9b9fa1c..f620a63c1 100644 --- a/tests/php/Forms/RequiredFieldsTest.php +++ b/tests/php/Forms/RequiredFieldsTest.php @@ -5,9 +5,6 @@ namespace SilverStripe\Forms\Tests; use SilverStripe\Dev\SapphireTest; use SilverStripe\Forms\RequiredFields; -/** - * @todo Test the validation method php() - */ class RequiredFieldsTest extends SapphireTest { diff --git a/tests/php/Forms/TimeFieldTest.php b/tests/php/Forms/TimeFieldTest.php index 153599144..54cc6f465 100644 --- a/tests/php/Forms/TimeFieldTest.php +++ b/tests/php/Forms/TimeFieldTest.php @@ -59,7 +59,6 @@ class TimeFieldTest extends SapphireTest $f = new TimeField('Time', 'Time'); $f->setHTML5(false); $f->setLocale('fr_FR'); - // TODO Find an hour format that's actually different $f->setValue('23:59'); $this->assertEquals($f->dataValue(), '23:59:00'); } diff --git a/tests/php/ORM/DBFieldTest.php b/tests/php/ORM/DBFieldTest.php index 8168137b5..cb1cb0ae7 100644 --- a/tests/php/ORM/DBFieldTest.php +++ b/tests/php/ORM/DBFieldTest.php @@ -117,8 +117,6 @@ class DBFieldTest extends SapphireTest $this->assertEquals(true, $boolean->prepValueForDB(1)); $this->assertEquals(true, $boolean->prepValueForDB('1')); - // @todo - Revisit Varchar to evaluate correct behaviour of nullifyEmpty - /* Varchar behaviour: nullifyifEmpty defaults to true */ $varchar = DBVarchar::create(); $this->assertEquals(0, $varchar->prepValueForDB(0)); diff --git a/tests/php/ORM/DataObjectLazyLoadingTest.php b/tests/php/ORM/DataObjectLazyLoadingTest.php index 7581bc46f..ad2ed19ab 100644 --- a/tests/php/ORM/DataObjectLazyLoadingTest.php +++ b/tests/php/ORM/DataObjectLazyLoadingTest.php @@ -95,8 +95,6 @@ class DataObjectLazyLoadingTest extends SapphireTest $teams = DataObject::get(Team::class); // query parent class $subteam1Lazy = $teams->find('ID', $subteam1->ID); - // TODO Fix hasField() to exclude *_Lazy - // $this->assertFalse($subteam1Lazy->hasField('SubclassDatabaseField_Lazy')); $this->assertTrue($subteam1Lazy->hasField('SubclassDatabaseField')); } diff --git a/tests/php/ORM/DataObjectTest.php b/tests/php/ORM/DataObjectTest.php index 8d65c03db..af99c2dec 100644 --- a/tests/php/ORM/DataObjectTest.php +++ b/tests/php/ORM/DataObjectTest.php @@ -830,9 +830,6 @@ class DataObjectTest extends SapphireTest $this->assertEquals($captain2->ID, $team1->Captain->ID); } - /** - * @todo Extend type change tests (e.g. '0'==NULL) - */ public function testChangedFields() { $obj = $this->objFromFixture(DataObjectTest\Player::class, 'captain1'); @@ -1140,9 +1137,6 @@ class DataObjectTest extends SapphireTest ); } - /** - * @todo Re-enable all test cases for field existence after behaviour has been fixed - */ public function testFieldExistence() { $teamInstance = $this->objFromFixture(DataObjectTest\Team::class, 'team1'); @@ -1290,9 +1284,6 @@ class DataObjectTest extends SapphireTest ); } - /** - * @todo Re-enable all test cases for field inheritance aggregation after behaviour has been fixed - */ public function testFieldInheritance() { $schema = DataObject::getSchema(); @@ -1633,12 +1624,6 @@ class DataObjectTest extends SapphireTest null, 'merge() with $overwriteWithEmpty overwrites empty fields on left object' ); - - // @todo test "left" priority flag - // @todo test includeRelations flag - // @todo test includeRelations in combination with overwriteWithEmpty - // @todo test has_one relations - // @todo test has_many and many_many relations } public function testPopulateDefaults() @@ -1919,7 +1904,6 @@ class DataObjectTest extends SapphireTest public function testManyManyUnlimitedRowCount() { $player = $this->objFromFixture(DataObjectTest\Player::class, 'player2'); - // TODO: What's going on here? $this->assertEquals(2, $player->Teams()->dataQuery()->query()->unlimitedRowCount()); } diff --git a/tests/php/ORM/DataQueryTest.php b/tests/php/ORM/DataQueryTest.php index 7e3fa6480..c2b7342aa 100644 --- a/tests/php/ORM/DataQueryTest.php +++ b/tests/php/ORM/DataQueryTest.php @@ -202,9 +202,6 @@ class DataQueryTest extends SapphireTest ); } - /** - * @todo Test paramaterised - */ public function testNestedGroups() { $dq = new DataQuery(DataQueryTest\ObjectA::class); diff --git a/tests/php/ORM/ManyManyThroughListTest/PolyItem.php b/tests/php/ORM/ManyManyThroughListTest/PolyItem.php index 5afdd87c2..df5364ea7 100644 --- a/tests/php/ORM/ManyManyThroughListTest/PolyItem.php +++ b/tests/php/ORM/ManyManyThroughListTest/PolyItem.php @@ -21,7 +21,6 @@ class PolyItem extends DataObject implements TestOnly /** * Placeholder for missing belongs_many_many for polymorphic relation * - * @todo Make this work for belongs_many_many * @return Generator|DataObject[] */ public function Objects() diff --git a/tests/php/ORM/PolymorphicHasManyListTest.php b/tests/php/ORM/PolymorphicHasManyListTest.php index 916166cee..e7211ef4b 100644 --- a/tests/php/ORM/PolymorphicHasManyListTest.php +++ b/tests/php/ORM/PolymorphicHasManyListTest.php @@ -10,8 +10,6 @@ use SilverStripe\ORM\Tests\DataObjectTest\Team; * Tests the PolymorphicHasManyList class * * @see PolymorphicHasManyList - * - * @todo Complete */ class PolymorphicHasManyListTest extends SapphireTest { diff --git a/tests/php/Security/MemberCsvBulkLoaderTest.php b/tests/php/Security/MemberCsvBulkLoaderTest.php index 723adcd46..e768f6125 100644 --- a/tests/php/Security/MemberCsvBulkLoaderTest.php +++ b/tests/php/Security/MemberCsvBulkLoaderTest.php @@ -104,7 +104,6 @@ class MemberCsvBulkLoaderTest extends SapphireTest DataObject::flush_and_destroy_cache(); $member = DataObject::get_by_id(Member::class, $memberID); - // TODO Direct getter doesn't work, wtf! $this->assertEquals(Security::config()->password_encryption_algorithm, $member->getField('PasswordEncryption')); $auth = new MemberAuthenticator(); $result = $auth->checkPassword($member, 'mypassword'); diff --git a/tests/php/Security/SecurityDefaultAdminTest.php b/tests/php/Security/SecurityDefaultAdminTest.php index 082e7895b..034f4a68d 100644 --- a/tests/php/Security/SecurityDefaultAdminTest.php +++ b/tests/php/Security/SecurityDefaultAdminTest.php @@ -21,8 +21,6 @@ class SecurityDefaultAdminTest extends SapphireTest { parent::setUp(); - // TODO Workaround to force database clearing with no fixture present, - // and avoid sideeffects from other tests if (!static::$tempDB->isUsed()) { static::$tempDB->build(); } diff --git a/tests/php/View/RequirementsTest.php b/tests/php/View/RequirementsTest.php index 388927233..a16cfa8eb 100644 --- a/tests/php/View/RequirementsTest.php +++ b/tests/php/View/RequirementsTest.php @@ -18,10 +18,6 @@ use SilverStripe\Dev\Deprecation; use SilverStripe\View\SSViewer; use SilverStripe\View\ThemeResourceLoader; -/** - * @todo Test that order of combine_files() is correct - * @todo Figure out how to clear the modified state of Requirements class - might affect other tests. - */ class RequirementsTest extends SapphireTest { diff --git a/tests/php/View/SSViewerTest.php b/tests/php/View/SSViewerTest.php index 487153e02..ce91527f3 100644 --- a/tests/php/View/SSViewerTest.php +++ b/tests/php/View/SSViewerTest.php @@ -2057,7 +2057,6 @@ EOC; public function testRequireCallInTemplateInclude() { - //TODO undo skip test on the event that templates ever obtain the ability to reference MODULE_DIR (or something to that effect) if (FRAMEWORK_DIR === 'framework') { $template = new SSViewer(['SSViewerTestProcess']); From 50aaf9a9d548e469ba49bd2d480fd69fb2e92730 Mon Sep 17 00:00:00 2001 From: Sabina Talipova Date: Tue, 31 Oct 2023 15:41:06 +1300 Subject: [PATCH 2/2] MNT Table header closed tag position --- tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php b/tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php index d7f350a36..51fc0085e 100644 --- a/tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php +++ b/tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php @@ -255,7 +255,7 @@ EOS $editor->setEditorConfig($restrictedConfig); $expectedHtmlString = '

standard text

Header'; - $htmlValue = '

standard text

Header
'; + $htmlValue = '

standard text

Header
'; $editor->setValue($htmlValue); $editor->saveInto($obj); $this->assertEquals($expectedHtmlString, $obj->Content, 'Table is not removed');