Core: The API and the database don't return the same count of discussions

[glowingblue]

Bug Report

Current Behavior
The API call /api/discussions?page[limit]=1000 returns 50 discussions.
The API call /api/discussions?page[limit]=-1 returns 100 discussions.
When I go directly to the database and execute the query SELECT count(*) FROM discussions. It returns 180.

Steps to Reproduce

1. Make an API call and compare the discussion count values from the API and the database.

Expected Behavior
The values should be the same.

Environment

• Flarum version: 0.1.0-beta.7
• PHP version: 7.0.27-0+deb9u1

[luceos]

@glowingblue why should both values be the same. The API filters the resultset eg:

• soft deleted discussions
• private discussions
• discussions under tags that are not included by default on the index

etc,etc.

[tobyzerner]

The API caps the number of results at 100. You'll need to make multiple API calls if you want to retrieve more discussions than that.

Actually, it's supposed to cap the number of results at 50, so that fact that you're able to get 100 is a bug. Will investigate further.

[glowingblue]

@luceos it's ok if it filters like that, no problem with that.

@tobscure ok, good to know, is that somewhere documented? We also tried to track it down in the code, we could find the $maxLimit = 50 part, that seems to cap it when setting the limit to something higher than that, e.g. ?page[limit]=1000, but not the 100 cap, could you give us a hint where that is defined (trying to understand the core code better here).

The idea was to use the API to build a more sophisticated version of the dashboard extension, mainly the stats part. As that extension directly goes to the database to get a count of discussion (for example), it seems inaccurate as it does not represent the same filtering as @luceos mentioned. Furthermore, wouldn't it be useful if the API had a way to return just the total count of Objects (where it makes sense). It seems unnecessary to fetch all discussions (including all attributes) just to figure out how many of them are there? What would be the best way to add something like that to the API? A new route + controller method that would allow something like /api/discussions/count?

[tobyzerner]

I can't reproduce this. /api/discussions?page[limit]=-1 is giving me zero discussions.

I would suggest caching the result of Discussion::whereVisibleTo(new Guest)->count() and adding it as an attribute to GET /api (ForumSerializer). Alternatively if it's admin-only you could add it to the admin JS payload (similar to what flarum-ext-statistics currently does).

[franzliedke]

Just a general remark: APIs are more useful in my experience when they add additional semantics on top of just being a way to do CRUD operations to a database table.

A dedicated endpoint (or attribute in the ForumSerializer) should be the better way forward here.

[glowingblue]

@tobscure @franzliedke like api/discussions/count?

[tobyzerner]

I think we should stick to JSON-API/REST semantics as much as possible until we absolutely have to diverge. I am in favour of a discussionsCount attribute in the ForumSerializer.

[glowingblue]

Going to look into it! And I guess there is good reasons for the hard cap or not allowing [limit=-1] to get all posts (thinking about large sets with the potential to take down the community)?

[tobyzerner]

Or I suppose adding a meta attribute to /api/discussions could work too 🤔

[glowingblue]

yap, saw that too, might be even better...

[franzliedke]

It depends on the intended use-case. If that count should match up with what you can retrieve via /api/discussions (i.e. no private discussions), then the meta attribute would make sense. If it's about the total count of rows in the database table (remember, that table in the API resource don't have to match up).