Bulk product import stalling at 66% when missing info #13001
Conversation
chahmedejaz
changed the title
| end | ||
|
|
||
| check_on_hand_nil(entry, new_variant) |
There was a problem hiding this comment.
- If the sheet doesn't have the
unitsoron_handpresent, then the variant is not saved due to model validation - After that, while assigning
on_handvalue in this method, the error is raised that the variant is not created first - So we need to make sure that the variant is created before calling this method
| @@ -76,7 +76,7 @@ def process_data(method) | |||
| begin | |||
| @importer.public_send("#{method}_entries") | |||
| rescue StandardError => e | |||
| render json: e.message, response: 500 | |||
| render plain: e.message, status: :internal_server_error | |||
There was a problem hiding this comment.
The UI does not render any error on the import loading screen and stalls because of the existing implementation of the render method. The string error message is being passed to the json option, which is not valid (maybe it was valid at the time of implementation), so the data was not present in the error response here, causing the UI to not show any errors.
This is now fixed such that the string message is being returned as plain text, and its status option is also set respectively, so the errors on the UI sshowcorrectly now
| @@ -30,13 +30,13 @@ def import | |||
| def validate_data | |||
| return unless process_data('validate') | |||
|
|
|||
| render json: @importer.import_results, response: 200 | |||
| render json: @importer.import_results | |||
There was a problem hiding this comment.
Apparently, it's not a valid option anymore (maybe). It implicitly returns 200 status.
chahmedejaz
force-pushed
the
bugfix/12973-product-import-never-completes
branch
from
b63300f to
80722aa
Compare
chahmedejaz
added
bug-s2
chahmedejaz
force-pushed
the
bugfix/12973-product-import-never-completes
branch
from
80722aa to
5d6ac43
Compare
|
The same spec is failing in master as well: https://github.com/openfoodfoundation/openfoodnetwork/actions/runs/12076014528/job/33676757352#step:10:72 |
|
The failing spec was introduced by another PR. |
- if the sheet doesn't have the units present, then the variant is not saved due to model validation - After that, while assigning on_hand value, error is raised that the variant is not created first - Now this commit makes sure that the variant is created before implementing above logic
- The caught errors do not get rendered to the UI because of incorrect rendering methods
filipefurtad0
force-pushed
the
bugfix/12973-product-import-never-completes
branch
from
5d6ac43 to
93a3130
Compare
filipefurtad0
added
pr-staged-fr
|
Hey @chahmedejaz, I could not reproduce the 66% upload bar, but I did notice that the bar does not progress and that there is a console error, if the Units field is left empty, on the CSV file:
After this PR, the file is uploaded successfully, and an informative error message is displayed to the customer:
If both On hand and On_demand fields are left empty, then this error message is displayed:
If all mandatory fields are filled in, then products are updated:
Looks great 🎉 |
What? Why?
What should we test?
unitsandon_handvalue presentRelease notes
Changelog Category (reviewers may add a label for the release notes):