@diosmosis opened this Issue on October 11th 2014 Member

As title. This is one of the main things stopping us from getting rid of twig. As long as data needs to be served w/ templates, there has to be PHP code and more specifically, controller code that uses a twig view.

Here is one possible way to remove the need for PHP controllers:

Create a special include directive, like piwik-include:

<div piwik-include="plugins/MyPlugin/mytemplate.html"
        piwik-api-my-data-attr="'method=MyPlugin.getMyData'"/>

The directive would get all attributes prefixed w/ piwik-api and do a bulk fetch for the API data that includes a request to get the template, eg,

method: API.bulkFetch,
urls: [
    'plugins/MyPlugin/mytemplate.html'
    '?method=MyPlugin.getmyData',
]

When the request is finished, the attribute values are replaces w/ the JSON encoded data of API responses.

@tsteur commented on October 12th 2014 Owner

BTW: We would get rid of Twig by using Angular routes #6037 . Once this is done all needed data would be requested via XHR. Would personally still let the browser fetch the HTML templates directly and let the server serve the HTML templates. Would be better for caching and reduce possible failures in case an API fails the template would be still served etc.

Would be awesome to get routes into Piwik and start removing twig

@diosmosis commented on October 13th 2014 Member

@tsteur Could you describe how routes would allow fetching data for directives? Or will directives have to call piwikApi manually? Btw, I was thinking about adding CliMulti support for API.bulkFetch which would decrease the amount of data loss should a single request fail and decrease the amount of AJAX requests created by the browser (which, if templates and all data will be retrieved via by AJAX, may go up a lot in the near future).

@tsteur commented on October 13th 2014 Owner

Data fetched would be done in Angular controller. In routes you can even define what data should be loaded/resolved before rendering the templates and directives, see for instance http://stackoverflow.com/a/11972028

@diosmosis commented on October 13th 2014 Member

That looks interesting, though it seems for individual directives, API requests would have to be done by the directives themselves? I can see how this would remove the need for twig, though.

I would still support making bulkFetch use CliMulti and be easy to use in JS to cut down on the amount of AJAX requests made.

@mattab commented on October 13th 2014 Owner

making bulkFetch use CliMulti and be easy to use in JS to cut down on the amount of AJAX requests made

original idea to use CliMulti for bulk but seems maybe wrong. Bulk is supported in API: http://developer.piwik.org/api-reference/reporting-api#advanced-users-send-multiple-api-requests-at-once so we could use this somehow.

IMHO number of Ajax requests is not a big problem as requests should be cached by browser with proper caching headers. if I understand correctly then we wouldn't need this and instead can use AngularJS routes #6037 - please reopen if i'm wrong :+1:

@diosmosis commented on October 13th 2014 Member

caching is a problem when getting requests through the API since, 1) all requests made by the piwik UI are POSTs 2) Piwik doesn't provide caching. caching for HTML templates is fine, I think.

I don't know what your first paragraph means. I was suggesting to use bulk requests (from API) and use CliMulti in Piwik so requests are handled concurrently instead of sequentially. Could be specified w/ async=1 query parameter.

@diosmosis commented on October 14th 2014 Member

@mattab ping

@mattab commented on October 14th 2014 Owner

1) So far it is by design that our API does not do caching since responses are supposed to be fast. Maybe we need to add caching though at some point if we start hitting API more and more from the UI.

What's the purpose of this ticket, is it to discuss how to get rid of twig? I'm not sure I get it, feel free to re-open if it's important and the issue discussed here won't be solved with AngularJS routes. cheers

@tsteur commented on October 14th 2014 Owner

Bulk requests can be used IMO if a directive has multiple resources to fetch to reduce the number of requests. They should not be combined over multiple directives to keep directives maintainable and independent. Not sure how many requests are sent max by one directive, maybe not so many?

Sending requests in parallel is basically already done by the browser (depends on the browser how many) so I do not really see a need to use CliMulti here. Especially since CliMulti doesn't work on all servers anyway (like not when using fpm, when using windows, ...) and it only sends a certain amount of requests in parallel.

Ideally, at some point we have a REST API which sends proper Expired / Not modified headers and we actually can cache requests in the browser. In this case single requests would be better as they can be better cached by the browser.

@diosmosis commented on October 14th 2014 Member

Supporting caching in the API fixes the issue I was thinking about. Although it would mean not using POSTs for every request.

@tsteur commented on October 14th 2014 Owner

Ideally with REST API we will use correct verbs and not always POST. Having a true REST API would be so nice for Angular usage as we would get some features just out of the box

@diosmosis commented on October 14th 2014 Member

I know, but POST was added for some security fix in the past having to do w/ token auths in the browser cache, so that will have to be dealt w/.

@mattab commented on October 16th 2014 Owner

that's a legit point: token_auth should be POSTed for advanced security (not leaking them in browser cache or on HTTP requests)... it will be interesting to see how it could work with REST and caching

@diosmosis commented on October 16th 2014 Member

I think maybe sessions can be used, and for some requests, the token auth is probably not necessary. For those, we can just switch to GET (and maybe put a check in piwikApi where an exception is thrown if the token_auth query param is used w/ the HTTP GET verb).

@tsteur commented on October 16th 2014 Owner

we will be able to workaround that... see for instance http://restcookbook.com/Basics/loggingin/ etc.
Most API's are based on REST and respect the proper HTTP methods ;)

Actually beside Piwik I can't think of any API that I've used in the last years that was not RESTful

@diosmosis commented on October 16th 2014 Member

Hoorah for oauth!

@mattab commented on October 16th 2014 Owner

hmac with use-once nonce sounds good, didn't hear of this idea before! oAuth also or instead would make sense too.

This Issue was closed on October 13th 2014
Powered by GitHub Issue Mirror