Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: Handle response errors status #240

Merged

Conversation

syphax-bouazzouni
Copy link
Contributor

@syphax-bouazzouni syphax-bouazzouni commented Dec 28, 2022

Context

Before ncbo/ontologies_api_ruby_client#13, the HTTP requests (update or create) return nil(ruby null) if successful and the response body if error.

After merging ncbo/ontologies_api_ruby_client#13, now all the HTTP would return a response, success, or error.

Issue

the old way of testing if the response is a success (response.nil?) will no more work because we don't return nil if success

Solution

Now the code will test the response status to know if it is a success or not

Changes

  • Harmonize the response error check with the response_error? helper
  • Update all the places were the the response success is tested

@jvendetti jvendetti marked this pull request as draft January 14, 2023 01:42
@jvendetti jvendetti marked this pull request as ready for review January 14, 2023 01:43
Copy link
Member

@jvendetti jvendetti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pull request Syphax. There's an issue with the create method in the SubmissionsController - I entered a comment with more details on line 25.

@syphax-bouazzouni
Copy link
Contributor Author

Hi, @jvendetti
Thanks for the time that you took for the reviews (and sorry for the troubles behind it)
I fixed the issue that you mentioned (see screenshot bellow)
image
image

@syphax-bouazzouni
Copy link
Contributor Author

syphax-bouazzouni commented Jan 16, 2023

My original goal is to reuse the error messages sent by the backend. In the case of Bioportal submission, there are only 2 possible errors: missing contact and upload files.

But in Agroportal we have many more fields in that form to test their validity. So it is necessary to reuse the backed errors message and not rewrite them in the front. Something like below
image

@syphax-bouazzouni syphax-bouazzouni force-pushed the pr/fix/response-error-handeling branch from 24efec1 to 2052c31 Compare January 18, 2023 15:16
@jvendetti jvendetti merged commit 5d6d164 into ncbo:master Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants