Skip to content
This repository has been archived by the owner on May 31, 2024. It is now read-only.

Users totalCount and removed one console.log #2035

Merged
merged 2 commits into from
Aug 21, 2018

Conversation

Apollinaire
Copy link
Member

No description provided.

@SachaG
Copy link
Contributor

SachaG commented Aug 21, 2018

I don't really get that change, if you don't want the totalCount couldn't you just not ask for it in the GraphQL fragment? Or are you thinking about the performance impact of making the extra db request?

@Apollinaire
Copy link
Member Author

This is unrelated to the discussion we had on Slack yesterday.

Here, I'm just re-enabling the total count on the Users resolver, which I forgot to add in my previous PR. Without this a datatable on the users collection would not be able to load more users than its limit (at least the material-ui version) because totalCount would be null.

For the record, here's what I said on Slack :

Currently, there are no access restrictions on the totalCount returned by the multi default resolver in the openCrud update. I think it is necessary, I want to be able to choose who has access to this or not.

I did not implement anything here regarding this subject, I just copied the pattern from the default mutators.

Sorry if this was confusing.

@SachaG
Copy link
Contributor

SachaG commented Aug 21, 2018

Oh ok sorry, I totally got confused. You just copied the default resolvers.

Btw the only reason why we're not using the default resolvers for users is that I didn't want the vulcan:users package to depend on vulcan:core since vulcan:core depends on vulcan:users

And the reason why default resolvers and mutations are in core instead of lib is because of #2037 btw.

@SachaG SachaG merged commit b67c928 into VulcanJS:devel Aug 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants