CiviCRM Community Forums (archive)

So the advantage of this approach as opposed to say making the api optionally return sql rather than a result array is that the FROM clauses etc would be more complex than a standard event.get?

The advantage of making it an extension of api.get would be that when we add ACLs to the api we would kill 2 birds with a massive big rock

Do you want to paste some examples?

we don't "need" as little php as possible, but so far, I like the idea of having to put a single template at the right place and get it working.

example of query:

https://github.com/TechToThePeople/civisualize/blob/master/queries/contacts.sql

For that one, we for instance would want to have a new contacts_in_group.sql that would only analyse the contacts belonging to a specific group.

Make sense?

X+

For reporting, I tend to agree that SQL is easier to work with than custom API functions. The nice thing about the described {crmSQL} helper (or an analogous API) is that the remoter caller doesn't actually write the SQL: the SQL is always coded on the server-side in a file.

Other random comments...

1. I'd personally find it easier to read if the parameter position/type were swapped, eg "1_integer=123" instead of "integer_1=123". To my eye, "integer_1" looks like "integer #1", which makes me think "integer #1" and "string #1" are two totally different things (which could coexist).  But that's probably not the intention.

2. How should one handle file-lookup with multiple extensions? It seems like you'd need to know the name of the extension so that you can lookup SQL from the right folder. e.g.

{crmSQL ext="org.example.mymodule" file="my-query.sql"}

// ...or use the same approach {ts}...

{crmScope extensionKey="org.example.mymodule"}
  {crmSQL file="my-query.sql"}
{/crmScope}

3. This is probably wishful thinking, but it would make even more sense to me to use named parameters with types declared in the .sql file, e.g. one way might be https://gist.github.com/totten/d4206ca9a5cc715c66c0 (Note: That example makes two passes through the template -- once to get list of variables/types and once to evaluate SQL. There might be a simpler way, but I just wanted to get something readable.)

I'm not too sure why, but my feeling is that we increase the attack surface (now everyone having an access to the api can much more easily try to sql/inject..) without additional benefits.

This being said, it would be trivial to add an api wrapper to provide an access to the sql queries later on if we realise we need it.

{crmSQL} seems to have two modes: it can accept raw SQL, or it can use a named SQL query (file). Raw SQL seems OK for server-side code but is a non-starter for remote APIs. The named SQL queries could be OK for an API (given suitable authorization logic). FWIW, I'd be less worried about injections stemming from user-supplied data with a REST call than with Smarty *.tpl files.

There's a general limitation with authorizations in both the current implementation of "/civicrm/dataviz" and (as I understand) the proposed APIs: in "/civicrm/dataviz", any user with permission "access CiviCRM" can request any dataviz .tpl, which means they can probably get the data for any named-query (assuming that the named-queries are actually used). If one did a simple API wrapper for the named-queries, then it would probably have the same limitation (eg anyone with "access CiviCRM" could get the query results). Not a problem for small orgs / simple installations, so maybe not a priority for you now, but could be an issue with bigger orgs. But maybe there's a simple solution -- like a adding a new Smarty function "{crmAuthorize perm='access CiviEvent'}" (which throws an exception if you lack permission).

So far, I only needed template lookup with multiple extensions (and it works fine, but with the limitation that everything is under the same url namespace /civicrm/dataviz )

side node: May be we should reserve /civicrm/tld.domain.extension (two dots in the template name part of the url) as a way to uniquely identify the tpl you want so several extensions can offer a dashboard

We could offer the optional ext param and by default lookup for the sql in the same extension as the template file is.

Aah, I hadn't read the code. I understand now -- when {crmSQL} loads a file, it searches the include_path, and each extension adds itself to the include_path. So there's some risk of collision (where two extensions have the same query-file), but that's easily managed downstream (eg by giving files distinctive names).

I'll move the discussion about templating the sql in a new thread

Will join you over there...

Yeah I can see the use case for crmAuthorize