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

RESTful help #2165

Merged
merged 23 commits into from
Sep 2, 2019
Merged

RESTful help #2165

merged 23 commits into from
Sep 2, 2019

Conversation

jim-parry
Copy link
Contributor

@jim-parry jim-parry commented Aug 27, 2019

This includes two classes to make building a RESTful API easier:

  • ResourceController, that you would extend, implmenting only those HTTP-verb based methods of interest; this is intended to accept JSON/XML and to return same - no views. This could be appropriate as a backend REST API for a resource, called through AJAX.
  • ResourcePresenter - a controller shadowing ResourceController, but intended for views & form handling only, so no JSON/XML exchanged. This would be useful for building server-side maintenance for a resource, with minimal reliance on a rich front-end.

When time permits (a future PR), I will submit/propose a RESTFIlter, to automatically translate between JSON & XML data in & out.

I developed this about six months ago, with & for my students, and am only now getting around to proving its correctness with unit testing. That has been problematic, as you might gather from the code. Not to worry - it is getting cleaner every day :)

Completes #1765

@jim-parry
Copy link
Contributor Author

@MGatner Here ya go - some ideas. Not "proven" yet, so user beware!

@MGatner
Copy link
Member

MGatner commented Aug 27, 2019

Wow thanks! I’ll look through this tonight - I like what I see.

system/RESTful/ResourcePresenter.php Outdated Show resolved Hide resolved
system/RESTful/ResourcePresenter.php Outdated Show resolved Hide resolved
system/Router/RouteCollection.php Outdated Show resolved Hide resolved
system/Router/RouteCollection.php Outdated Show resolved Hide resolved
system/Test/NakedFeatureTestCase.php Outdated Show resolved Hide resolved
@jim-parry
Copy link
Contributor Author

@andreportaro Those changes are done automatically by the git commit pre-hook, and that is what needs fixing. My PR probably has more formatting changes than normal, because it has been in the works for so long.
I would not worry about the formatting changes, because they can get automatically changed in any PR with the commit hook :(

@andreportaro
Copy link
Contributor

@andreportaro Those changes are done automatically by the git commit pre-hook, and that is what needs fixing. My PR probably has more formatting changes than normal, because it has been in the works for so long.
I would not worry about the formatting changes, because they can get automatically changed in any PR with the commit hook :(

I see! Is that pre-commit hook versioned anywhere?

@jim-parry
Copy link
Contributor Author

The pre commit hook runs our standard code sniffer, codeigniter4/codeigniter4-standard. My guess is that is the appropriate place to change things.

The good news is that once that is updated, the formatting changes will percolate through the system automatically, over time :) It may make for some funny PRs for a bit, with lots of formatting changes, but it will settle down :)

@jim-parry
Copy link
Contributor Author

Not ready - the code coverage has revealed a flaw in the presenter routing.

@jim-parry
Copy link
Contributor Author

Hmm - ControllerTesterTest is failing travis-ci, and I don't see why.
I don't think I touched anything that might affect that :(
Otherwise, looking good :)

@jim-parry jim-parry changed the title WIP RESTful help RESTful help Aug 31, 2019
@jim-parry
Copy link
Contributor Author

Fixed the travis-ci issue with phpunit annotations to run resource & controller tests in separate processes, preventing test cross-contamination.

@lonnieezell lonnieezell merged commit 2b3981b into codeigniter4:develop Sep 2, 2019
{
$this->modelName = $which;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should'nt we force the name and object to be cleared here once something has been introduced?
It's just that if we are assigning the model for the first time, everything is fine, but if we are changing the model it will not work well anymore because we will have the class name of the previous model and the new object or the previous object and with the name of the new class.
Maybe something like this:
if (is_object($which))
{
$this->model = $which;
$this->modelName = null;
}
else
{
$this->model = null;
$this->modelName = $which;
}

@MGatner MGatner mentioned this pull request Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants