search

onaio / kaznet-web

a tasking application built on top of Onadata
Apache License 2.0
4 stars 1 forks source link

Fix issue where getting an unsuccessful response would delete all XForms #377

Closed DavisRayM closed 4 years ago

DavisRayM commented 4 years ago

Closes #376

moshthepitt commented 4 years ago

@DavisRayM Looks like we are assuming that if we ever receive an empty list of projects from Ona then something is wrong. Is this a valid assumption?

Is there a way to actually check the response that we get from Ona i.e. is its not a 200 then abort

moshthepitt commented 4 years ago

@DavisRayM also, do we need to do this for forms and submissions?

DavisRayM commented 4 years ago

@DavisRayM Looks like we are assuming that if we ever receive an empty list of projects from Ona then something is wrong. Is this a valid assumption?

Is there a way to actually check the response that we get from Ona i.e. is its not a 200 then abort

That was my assumption at the time, probably not the best assumption.

Currently we have no way to check the status_code for a request in most of the API functions since our request function here, doesn't return it. We could check and make sure it's not a None and continue the process.

DavisRayM commented 4 years ago

really nervous and curious about existing tests not needing an update @DavisRayM

Tests are currently failing, didn't try them out locally. I'll fix them

DavisRayM commented 4 years ago

@moshthepitt Fixed the failing tests but the tests that broke only broke due to a weird thing I was doing.... not sure if I should add more tests for this or not.. Any suggestion on what else should be tested within this work flow ?

moshthepitt commented 4 years ago

@DavisRayM do we need to do this for sync_deleted_instances as well?

DavisRayM commented 4 years ago

@DavisRayM do we need to do this for sync_deleted_instances as well?

We currently don't need to make any code change to it. But we do need a test to make sure it never happens.

DavisRayM commented 4 years ago

@DavisRayM do we need to do this for sync_deleted_instances as well?

We currently don't need to make any code change to it. But we do need a test to make sure it never happens.

Added a test for this