TAN-1794 - FormSync / XLSX - return async errors #7837
Conversation
| @@ -3,13 +3,14 @@ | |||
| module BulkImportIdeas | |||
| class IdeaImportJob < ApplicationJob | |||
| self.priority = 60 | |||
| perform_retries false | |||
There was a problem hiding this comment.
Didn't think we should be retrying these jobs - if they fail once they're likely to fail again
…rrors' into TAN-1794-formsync-return-async-errors
|
There was a problem hiding this comment.
Maybe I would prefer if last_error_message returned a nice and clean string that can be immediately used by FE.
Because these .split(': '), split('#'), and startsWith('bulk_import_') seem very BE and Ruby specific. It could be properly formatted in the serializer. We could also more easily reuse this logic in back/engines/commercial/admin_api/app/controllers/admin_api/jobs_controller.rb if needed.
But it's not a blocker, up to you.
I've just updated this logic to the backend. There is now a method in the controller to return a generic 'uncaught_error' if it's not a recognised error string. Do you want to take a quick look @alexander-cit ? |
|
|
||
| // When a bulk import error - there will be additional params | ||
| const splitErrorParams = splitError[1].split('#'); | ||
| const error_string = job.attributes.last_error_message; |
There was a problem hiding this comment.
Could it be camel-cased as other variables?
const errorString = job.attributes.last_error_message;| const splitErrorParams = error_string.split('#'); | ||
| if (splitErrorParams.length > 1) { | ||
| const params = JSON.parse(splitErrorParams[1]) as JobErrorParams; |
There was a problem hiding this comment.
Maybe these split and parse could also be done on the BE. The fact that we send to the FE JSON (HTTP response) with another JSON inside (these error params) seems a bit too complicated.
But it's definitely not a requirement. Feel free to merge!
…rrors' into TAN-1794-formsync-return-async-errors
# Conflicts: # front/app/translations/admin/ar-SA.json # front/app/translations/admin/da-DK.json # front/app/translations/admin/de-DE.json # front/app/translations/admin/en-CA.json # front/app/translations/admin/en-GB.json # front/app/translations/admin/en-IE.json # front/app/translations/admin/es-CL.json # front/app/translations/admin/es-ES.json # front/app/translations/admin/fi-FI.json # front/app/translations/admin/fr-BE.json # front/app/translations/admin/fr-FR.json # front/app/translations/admin/hr-HR.json # front/app/translations/admin/lv-LV.json # front/app/translations/admin/nb-NO.json # front/app/translations/admin/nl-BE.json # front/app/translations/admin/nl-NL.json # front/app/translations/admin/pl-PL.json # front/app/translations/admin/pt-BR.json # front/app/translations/admin/sr-Latn.json # front/app/translations/admin/sr-SP.json # front/app/translations/admin/sv-SE.json # front/app/translations/admin/tr-TR.json
Errors now look like this - I've tweaked text to make it clearer. Second image shows errors we don't specifically catch - but don't expect these to happen often.
Could not get it to only throw errors for those ideas that have failed - this would be too big a refactor right now.
Changelog
Changed