wizbots / labtab

0 stars 0 forks source link

Wizchips bug discovered #99

Closed alexey-ku77 closed 5 years ago

alexey-ku77 commented 5 years ago

I have checked code base of the project, with intent to find a bug, which still hit backend with attempts to add more than one wizchip. Let me remind you that we had issue with wizchips here #87.

I think that problem happens when data is synced here - org.wizbots.labtab.service.LabTabSyncService#syncWizchips.

https://github.com/wizbots/labtab/blob/96387f5c5203e54eb9ddabbc9502bce6ab79ad2e/app/src/main/java/org/wizbots/labtab/service/LabTabSyncService.java#L210-L227 Here request are made to add or withdraw wizchips. Instead set operation(REST API) should be used. Also I think that online and offline wizchips combined wrong way. As I understand online chips are absolute value, while offline is a delta to online wizchips. Instead I suggest to make them both absolute, and during sync do set operation when online(last returned value by server) value is different from offline value. This approach should give stable results.

kapoorutd commented 5 years ago

We dubbed the app and find the reason why kids offline wizchips are not updating on server, actually the add and withdraw wizchips apis are rejecting the request if the offline wizchips count are grater than one, so suppose if the offline wizchips count for a student is grater than 1 or less than -1 then there wizchips never gets updated on server, every time app request is rejected by server saying "Attempt to apply more than one wizchip"

Example 1: http://test.wizbots.com/api/students/wizchips/withdraw?students=59a6dd120d12a911f0a85c11&wizchips=2

Response{protocol=http/1.1, code=400, message=BAD REQUEST, url=http://test.wizbots.com/api/students/wizchips/withdraw?students=59a6dd120d12a911f0a85c11&wizchips=2}

Example 2: http://test.wizbots.com/api/students/wizchips/add?students=5a56916b0d12a92709c99e4e&wizchips=2

Response{protocol=http/1.1, code=400, message=BAD REQUEST, url=http://test.wizbots.com/api/students/wizchips/add?students=5a56916b0d12a92709c99e4e&wizchips=2}

alexey-ku77 commented 5 years ago

@kapoorutd if you recall we had a bug that caused excessive amount of wizchips being added to student account. So we had to restrict that api to single wizchip, as it intended initially(when plus or minus is pressed - appropriate request is made). Labtab need to be fixed to do proper math with wizchips and use set operation. Thus please reread my comment above. If something looks not very clear to you, I will try to help you with that. So you can make it.

kapoorutd commented 5 years ago

011d1e74618c8e5f63ad0010c325122e906b2e29

Implemented Set wizchip api only for offline wizchips syncing.