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

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
7 changes: 0 additions & 7 deletions src/DataFormatter.php
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Open ticket: #113

Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ abstract class DataFormatter
* ($has_one, $has_many, $many_many).
* Set to "0" to disable relation output.
*
* @todo Support more than one nesting level
*
* @var int
*/
public $relationDepth = 1;
Expand Down Expand Up @@ -290,9 +288,6 @@ public function getTotalSize()
* Returns all fields on the object which should be shown
* in the output. Can be customised through {@link self::setCustomFields()}.
*
* @todo Allow for custom getters on the processed object (currently filtered through inheritedDatabaseFields)
* @todo Field level permission checks
*
* @param DataObject $obj
* @return array
*/
Expand All @@ -303,7 +298,6 @@ protected function getFieldsForObj($obj)
// if custom fields are specified, only select these
if (is_array($this->customFields)) {
foreach ($this->customFields as $fieldName) {
// @todo Possible security risk by making methods accessible - implement field-level security
if (($obj->hasField($fieldName) && !is_object($obj->getField($fieldName)))
|| $obj->hasMethod("get{$fieldName}")
) {
Expand All @@ -318,7 +312,6 @@ protected function getFieldsForObj($obj)

if (is_array($this->customAddFields)) {
foreach ($this->customAddFields as $fieldName) {
// @todo Possible security risk by making methods accessible - implement field-level security
if ($obj->hasField($fieldName) || $obj->hasMethod("get{$fieldName}")) {
$dbFields[$fieldName] = $fieldName;
}
Expand Down
3 changes: 0 additions & 3 deletions src/DataFormatter/FormEncodedDataFormatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
* curl -X PUT -d "Name=This is an updated record" http://host/api/v1/(DataObject)/1
* </code>
*
* @todo Format response form encoded as well - currently uses XMLDataFormatter
*
* @author Cam Spiers <camspiers at gmail dot com>
*/
Expand All @@ -37,7 +36,5 @@ public function convertStringToArray($strData)
$postArray = array();
parse_str($strData ?? '', $postArray);
return $postArray;
//TODO: It would be nice to implement this function in Convert.php
//return Convert::querystr2array($strData);
}
}
1 change: 0 additions & 1 deletion src/DataFormatter/JSONDataFormatter.php
Copy link
Contributor Author

@sabina-talipova sabina-talipova Oct 16, 2023

Choose a reason for hiding this comment

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

Open ticket: #112

Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ class JSONDataFormatter extends DataFormatter
{
/**
* @config
* @todo pass this from the API to the data formatter somehow
*/
private static $api_base = "api/v1/";

Expand Down
1 change: 0 additions & 1 deletion src/DataFormatter/XMLDataFormatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ class XMLDataFormatter extends DataFormatter

/**
* @config
* @todo pass this from the API to the data formatter somehow
*/
private static $api_base = "api/v1/";

Expand Down
28 changes: 0 additions & 28 deletions src/RestfulServer.php
Copy link
Contributor Author

@sabina-talipova sabina-talipova Oct 16, 2023

Choose a reason for hiding this comment

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

Original file line number Diff line number Diff line change
Expand Up @@ -22,23 +22,6 @@
* Relies on serialization/deserialization into different formats provided
* by the DataFormatter APIs in core.
*
* @todo Implement PUT/POST/DELETE for relations
* @todo Access-Control for relations (you might be allowed to view Members and Groups,
* but not their relation with each other)
* @todo Make SearchContext specification customizeable for each class
* @todo Allow for range-searches (e.g. on Created column)
* @todo Filter relation listings by $api_access and canView() permissions
* @todo Exclude relations when "fields" are specified through URL (they should be explicitly
* requested in this case)
* @todo Custom filters per DataObject subclass, e.g. to disallow showing unpublished pages in
* SiteTree/Versioned/Hierarchy
* @todo URL parameter namespacing for search-fields, limit, fields, add_fields
* (might all be valid dataobject properties)
* e.g. you wouldn't be able to search for a "limit" property on your subclass as
* its overlayed with the search logic
* @todo i18n integration (e.g. Page/1.xml?lang=de_DE)
* @todo Access to extendable methods/relations like SiteTree/1/Versions or SiteTree/1/Version/22
* @todo Respect $api_access array notation in search contexts
*/
class RestfulServer extends Controller
{
Expand Down Expand Up @@ -119,7 +102,6 @@ public function init()
{
/* This sets up SiteTree the same as when viewing a page through the frontend. Versioned defaults
* to Stage, and then when viewing the front-end Versioned::choose_site_stage changes it to Live.
* TODO: In 3.2 we should make the default Live, then change to Stage in the admin area (with a nicer API)
*/
if (class_exists(SiteTree::class)) {
singleton(SiteTree::class)->extend('modelascontrollerInit', $this);
Expand Down Expand Up @@ -262,8 +244,6 @@ public function index(HTTPRequest $request)
* - static $api_access must be set. This enables the API on a class by class basis
* - $obj->canView() must return true. This lets you implement record-level security
*
* @todo Access checking
*
* @param string $className
* @param int $id
* @param string $relation
Expand Down Expand Up @@ -319,7 +299,6 @@ protected function getHandler($className, $id, $relationName)
return $this->notFound();
}

// TODO Avoid creating data formatter again for relation class (see above)
$responseFormatter = $this->getResponseDataFormatter($obj->dataClass());
}
} else {
Expand Down Expand Up @@ -362,8 +341,6 @@ protected function getHandler($className, $id, $relationName)
* an existing query object (mostly a component query from {@link DataObject})
* with search clauses.
*
* @todo Allow specifying of different searchcontext getters on model-by-model basis
*
* @param string $className
* @param array $params
* @return SS_List
Expand Down Expand Up @@ -556,9 +533,6 @@ protected function putHandler($className, $id)
/**
* Handler for object append / method call.
*
* @todo Posting to an existing URL (without a relation)
* current resolves in creatig a new element,
* rather than a "Conflict" message.
*/
protected function postHandler($className, $id, $relation)
{
Expand Down Expand Up @@ -678,7 +652,6 @@ protected function updateDataObject($obj, $formatter)
$data[$newkey] = $value;
}

// @todo Disallow editing of certain keys in database
$data = array_diff_key($data ?? [], ['ID', 'Created']);

$apiAccess = singleton($className)->config()->api_access;
Expand Down Expand Up @@ -866,7 +839,6 @@ protected function authenticate()

/**
* Return only relations which have $api_access enabled.
* @todo Respect field level permissions once they are available in core
*
* @param string $class
* @param Member $member
Expand Down
7 changes: 0 additions & 7 deletions tests/unit/JSONDataFormatterTest.php
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Open ticket: #111

Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,6 @@
use SilverStripe\Dev\SapphireTest;
use SilverStripe\RestfulServer\DataFormatter\JSONDataFormatter;

/**
*
* @todo Test Relation getters
* @todo Test filter and limit through GET params
* @todo Test DELETE verb
*
*/
class JSONDataFormatterTest extends SapphireTest
{
protected static $fixture_file = 'JSONDataFormatterTest.yml';
Expand Down
10 changes: 0 additions & 10 deletions tests/unit/RestfulServerTest.php
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Open ticket: #111

Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,6 @@
use SilverStripe\Core\Config\Config;
use SilverStripe\RestfulServer\DataFormatter\XMLDataFormatter;

/**
*
* @todo Test Relation getters
* @todo Test filter and limit through GET params
* @todo Test DELETE verb
*
*/
class RestfulServerTest extends SapphireTest
{
protected static $fixture_file = 'RestfulServerTest.yml';
Expand Down Expand Up @@ -101,7 +94,6 @@ public function testAuthenticatedGET()
$thing1 = $this->objFromFixture(RestfulServerTestSecretThing::class, 'thing1');
$comment1 = $this->objFromFixture(RestfulServerTestComment::class, 'comment1');

// @todo create additional mock object with authenticated VIEW permissions
$urlSafeClassname = $this->urlSafeClassname(RestfulServerTestSecretThing::class);
$url = "{$this->baseURI}/api/v1/$urlSafeClassname/" . $thing1->ID;
$response = Director::test($url, null, null, 'GET');
Expand Down Expand Up @@ -158,7 +150,6 @@ public function testGETRelationshipsXML()
$rating1 = $this->objFromFixture(RestfulServerTestAuthorRating::class, 'rating1');
$rating2 = $this->objFromFixture(RestfulServerTestAuthorRating::class, 'rating2');

// @todo should be set up by fixtures, doesn't work for some reason...
$author1->Ratings()->add($rating1);
$author1->Ratings()->add($rating2);

Expand Down Expand Up @@ -187,7 +178,6 @@ public function testGETRelationshipsWithAlias()
$author1 = $this->objFromFixture(RestfulServerTestAuthor::class, 'author1');
$rating1 = $this->objFromFixture(RestfulServerTestAuthorRating::class, 'rating1');

// @todo should be set up by fixtures, doesn't work for some reason...
$author1->Ratings()->add($rating1);

$urlSafeClassname = $this->urlSafeClassname(RestfulServerTestAuthor::class);
Expand Down