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 #215

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions client/src/bundles/PagesDueForReview.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import jQuery from 'jquery';

/**
* @todo Re-validate this with Subsites
*/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's no longer actual issue

jQuery.entwine('ss', ($) => {
// Hide all owner dropdowns except the one for the current subsite
function showCorrectSubsiteIDDropdown(value) {
Expand Down
4 changes: 0 additions & 4 deletions src/Extensions/ContentReviewCMSExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,6 @@ protected function findRecord($data)
* Check if the current request has a X-Formschema-Request header set.
* Used by conditional logic that responds to validation results
*
* @todo Remove duplication. See https://github.com/silverstripe/silverstripe-admin/issues/240
*
* @return bool
*/
protected function getSchemaRequested()
Expand All @@ -142,8 +140,6 @@ protected function getSchemaRequested()
/**
* Generate schema for the given form based on the X-Formschema-Request header value
*
* @todo Remove duplication. See https://github.com/silverstripe/silverstripe-admin/issues/240
*
* @param string $schemaID ID for this schema. Required.
* @param Form $form Required for 'state' or 'schema' response
* @param ValidationResult $errors Required for 'error' response
Expand Down
2 changes: 0 additions & 2 deletions src/Reports/PagesDueForReviewReport.php
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@ public function sourceRecords($params = [], $sort = null, $limit = null)
} else {
// Review date before
if (!empty($params['ReviewDateBefore'])) {
// TODO Get value from DateField->dataValue() once we have access to form elements here
$nextReviewUnixSec = strtotime(
' + 1 day',
strtotime($params['ReviewDateBefore'] ?? '')
Expand All @@ -194,7 +193,6 @@ public function sourceRecords($params = [], $sort = null, $limit = null)

// Review date after
if (!empty($params['ReviewDateAfter'])) {
// TODO Get value from DateField->dataValue() once we have access to form elements here
$records = $records->where(
sprintf(
"\"NextReviewDate\" >= '%s'",
Expand Down
34 changes: 0 additions & 34 deletions tests/php/ContentReviewBaseTest.php
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

use SilverStripe\Dev\FunctionalTest;
use SilverStripe\CMS\Model\SiteTree;
// @todo add translatable namespace
use Translatable;
Copy link
Contributor Author

Choose a reason for hiding this comment

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


/**
* Extend this class when writing unit tests which are compatible with other modules.
Expand All @@ -17,36 +15,4 @@ abstract class ContentReviewBaseTest extends FunctionalTest
* @var bool
*/
protected $translatableEnabledBefore;

protected function setUp(): void
{
parent::setUp();

/*
* We set the locale for pages explicitly, because if we don't, then we get into a situation
* where the page takes on the tester's (your) locale, and any calls to simulate subsequent requests
* (e.g. $this->post()) do not seem to get passed the tester's locale,
* but instead fallback to the default locale.
*
* So we set the pages locale to be the default locale, which will then match any subsequent requests.
*
* If creating pages in your unit tests (rather than reading from the fixtures file), you must explicitly call
* self::compat() on the page, for the same reasons as above.
*/
if (class_exists(Translatable::class)) {
$this->translatableEnabledBefore = SiteTree::has_extension(Translatable::class);
SiteTree::remove_extension(Translatable::class);
}
}

protected function tearDown(): void
{
if (class_exists(Translatable::class)) {
if ($this->translatableEnabledBefore) {
SiteTree::add_extension(Translatable::class);
}
}

parent::tearDown();
}
}