nomad-nmr / nomad-server

Server side code for NOMAD system
https://www.nomad-nmr.uk/
GNU Affero General Public License v3.0
3 stars 2 forks source link

Enable accounting calculation for all users #120

Closed tomlebl closed 4 months ago

tomlebl commented 4 months ago

In admin/accounts, if "Users" switch is on, drop down menu to select group is enabled. Add "--all--" option in the list. If "--al--" option is selected than the backend should return data for all users.

tomlebl commented 4 months ago

@hunxjunedo This one could be for you. If you want to take this one on then let me know. I will assign it to you and provide a DB snapshot that you will need.

hunxjunedo commented 4 months ago

Sure It will be great to work on this.

tomlebl commented 4 months ago

@hunxjunedo Drop me an e-mail on nomad@st-andrews.ac.uk. The current DB dump is too big to be uploaded here. It will also reset the backdoor admin password. So, I will need to give you credentials that you can log in.

hunxjunedo commented 4 months ago

if I'm not wrong, this logic is already handled by the backend: When the a groupid isnt sent it traverses thru all the groups and therefore all the active users right ?
image

EDIT: I think there's something wrong with it, it doesn't return data of all users, instead it returns the data grouped by groupnames. Please guide me here.

tomlebl commented 4 months ago

This is correct. If groupId is undefined then all experiments are accounted and each row in the table corresponds to individual groups. If groupId is defined then only experiments recorded for selected group are accounted and each row of the table corresponds to individual users that belong to the selected group. The new feature is to add option that would send request with groupId defined "---All---" then you would account all experiments but each row would correspond to a user and thus table would show all users that have acquired an experiment in the given period of time. The feature would be useful for a smaller labs that has only handful users and groups. I hope that makes sense. If not keep asking.

hunxjunedo commented 4 months ago

That makes sense, one more thing: do we need to introduce pagination for now as there may be thousands of users ? Or that can be addressed in a separate PR ?

EDIT: this is really needed, and there's now way around, I suggest if we are implementing pagination, we should extend it to the group-specific accounting as well.I have planned this approach: first we get the users from the users collection according to the page settings (and groupid if its not --all-- option). Then we search experiments and claims for those users, then doing the rest of the calculations. This may significantly improve the response times.

tomlebl commented 4 months ago

I would not use pagination. Firstly, --all-- option will likely be used only for systems with small number of users and if group is defined or calculation is on the group level the table is rather small. Secondly, I want to export all results into .csv files and pagination would mess that up. That could be another task.

There is an important reason why the calculations are done the way they are done. Don't change that! Your suggested calculation would be quicker but could be inaccurate/wrong. Users can move from one group to another. So, when you calculate the user could be in a different group than it was in time when the experiment was done. I have been down that way. Long resp time is not a problem.

hunxjunedo commented 4 months ago

noted, so I'm going without any pagination, however will have to use a smaller dataset to test, since its taking forever to respond with this dump.

tomlebl commented 4 months ago

If you set the dates and calculate only since beginning of July or just the month of June it should be quicker.

tomlebl commented 4 months ago

@hunxjunedo It works fine. Well done. Thanks