From 91fd2b5d235e3ec1dbe23d23a8aa2e35d9a55d05 Mon Sep 17 00:00:00 2001 From: Lee Tickett <lee@tickett.net> Date: Tue, 27 Jul 2021 18:09:07 +0000 Subject: [PATCH] Render CSV parsing errors We need to handle translation of errors returned by the papaparse csv parsing library and package up a reusable component for displaying them. This can be resued in the upcoming diff viewer. Changelog: added --- .../javascripts/blob/csv/csv_viewer.vue | 13 +++--- .../components/papa_parse_alert.vue | 44 +++++++++++++++++++ locale/gitlab.pot | 18 ++++++++ spec/frontend/blob/csv/csv_viewer_spec.js | 13 +++--- .../components/papa_parse_alert_spec.js | 44 +++++++++++++++++++ 5 files changed, 119 insertions(+), 13 deletions(-) create mode 100644 app/assets/javascripts/vue_shared/components/papa_parse_alert.vue create mode 100644 spec/frontend/vue_shared/components/papa_parse_alert_spec.js diff --git a/app/assets/javascripts/blob/csv/csv_viewer.vue b/app/assets/javascripts/blob/csv/csv_viewer.vue index 050f2785d9a3..1f9d20a487f2 100644 --- a/app/assets/javascripts/blob/csv/csv_viewer.vue +++ b/app/assets/javascripts/blob/csv/csv_viewer.vue @@ -1,11 +1,12 @@ <script> -import { GlAlert, GlLoadingIcon, GlTable } from '@gitlab/ui'; +import { GlLoadingIcon, GlTable } from '@gitlab/ui'; import Papa from 'papaparse'; +import PapaParseAlert from '~/vue_shared/components/papa_parse_alert.vue'; export default { components: { + PapaParseAlert, GlTable, - GlAlert, GlLoadingIcon, }, props: { @@ -17,7 +18,7 @@ export default { data() { return { items: [], - errorMessage: null, + papaParseErrors: [], loading: true, }; }, @@ -26,7 +27,7 @@ export default { this.items = parsed.data; if (parsed.errors.length) { - this.errorMessage = parsed.errors.map((e) => e.message).join('. '); + this.papaParseErrors = parsed.errors; } this.loading = false; @@ -40,9 +41,7 @@ export default { <gl-loading-icon class="gl-mt-5" size="lg" /> </div> <div v-else> - <gl-alert v-if="errorMessage" variant="danger" :dismissible="false"> - {{ errorMessage }} - </gl-alert> + <papa-parse-alert v-if="papaParseErrors.length" :papa-parse-errors="papaParseErrors" /> <gl-table :empty-text="__('No CSV data to display.')" :items="items" diff --git a/app/assets/javascripts/vue_shared/components/papa_parse_alert.vue b/app/assets/javascripts/vue_shared/components/papa_parse_alert.vue new file mode 100644 index 000000000000..fa11661255fe --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/papa_parse_alert.vue @@ -0,0 +1,44 @@ +<script> +import { GlAlert } from '@gitlab/ui'; +import { s__ } from '~/locale'; + +export default { + components: { + GlAlert, + }, + i18n: { + genericErrorMessage: s__('CsvParser|Failed to render the CSV file for the following reasons:'), + MissingQuotes: s__('CsvParser|Quoted field unterminated'), + InvalidQuotes: s__('CsvParser|Trailing quote on quoted field is malformed'), + UndetectableDelimiter: s__('CsvParser|Unable to auto-detect delimiter; defaulted to ","'), + TooManyFields: s__('CsvParser|Too many fields'), + TooFewFields: s__('CsvParser|Too few fields'), + }, + props: { + papaParseErrors: { + type: Array, + required: false, + default: () => [], + }, + }, + computed: { + errorMessages() { + const errorMessages = this.papaParseErrors.map( + (error) => this.$options.i18n[error.code] ?? error.message, + ); + return new Set(errorMessages); + }, + }, +}; +</script> + +<template> + <gl-alert variant="danger" :dismissible="false"> + {{ $options.i18n.genericErrorMessage }} + <ul class="gl-mb-0!"> + <li v-for="error in errorMessages" :key="error"> + {{ error }} + </li> + </ul> + </gl-alert> +</template> diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 97f00c84cbb3..4e00a96868d6 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -9642,6 +9642,24 @@ msgstr "" msgid "Crowd" msgstr "" +msgid "CsvParser|Failed to render the CSV file for the following reasons:" +msgstr "" + +msgid "CsvParser|Quoted field unterminated" +msgstr "" + +msgid "CsvParser|Too few fields" +msgstr "" + +msgid "CsvParser|Too many fields" +msgstr "" + +msgid "CsvParser|Trailing quote on quoted field is malformed" +msgstr "" + +msgid "CsvParser|Unable to auto-detect delimiter; defaulted to \",\"" +msgstr "" + msgid "Current" msgstr "" diff --git a/spec/frontend/blob/csv/csv_viewer_spec.js b/spec/frontend/blob/csv/csv_viewer_spec.js index abb914b8f573..17973c709c14 100644 --- a/spec/frontend/blob/csv/csv_viewer_spec.js +++ b/spec/frontend/blob/csv/csv_viewer_spec.js @@ -1,8 +1,9 @@ -import { GlAlert, GlLoadingIcon, GlTable } from '@gitlab/ui'; +import { GlLoadingIcon, GlTable } from '@gitlab/ui'; import { getAllByRole } from '@testing-library/dom'; import { shallowMount, mount } from '@vue/test-utils'; import { nextTick } from 'vue'; -import CSVViewer from '~/blob/csv/csv_viewer.vue'; +import CsvViewer from '~/blob/csv/csv_viewer.vue'; +import PapaParseAlert from '~/vue_shared/components/papa_parse_alert.vue'; const validCsv = 'one,two,three'; const brokenCsv = '{\n "json": 1,\n "key": [1, 2, 3]\n}'; @@ -11,7 +12,7 @@ describe('app/assets/javascripts/blob/csv/csv_viewer.vue', () => { let wrapper; const createComponent = ({ csv = validCsv, mountFunction = shallowMount } = {}) => { - wrapper = mountFunction(CSVViewer, { + wrapper = mountFunction(CsvViewer, { propsData: { csv, }, @@ -20,7 +21,7 @@ describe('app/assets/javascripts/blob/csv/csv_viewer.vue', () => { const findCsvTable = () => wrapper.findComponent(GlTable); const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); - const findAlert = () => wrapper.findComponent(GlAlert); + const findAlert = () => wrapper.findComponent(PapaParseAlert); afterEach(() => { wrapper.destroy(); @@ -35,12 +36,12 @@ describe('app/assets/javascripts/blob/csv/csv_viewer.vue', () => { }); describe('when the CSV contains errors', () => { - it('should render alert', async () => { + it('should render alert with correct props', async () => { createComponent({ csv: brokenCsv }); await nextTick; expect(findAlert().props()).toMatchObject({ - variant: 'danger', + papaParseErrors: [{ code: 'UndetectableDelimiter' }], }); }); }); diff --git a/spec/frontend/vue_shared/components/papa_parse_alert_spec.js b/spec/frontend/vue_shared/components/papa_parse_alert_spec.js new file mode 100644 index 000000000000..9be2de17d017 --- /dev/null +++ b/spec/frontend/vue_shared/components/papa_parse_alert_spec.js @@ -0,0 +1,44 @@ +import { GlAlert } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; +import { nextTick } from 'vue'; +import PapaParseAlert from '~/vue_shared/components/papa_parse_alert.vue'; + +describe('app/assets/javascripts/vue_shared/components/papa_parse_alert.vue', () => { + let wrapper; + + const createComponent = ({ errorMessages } = {}) => { + wrapper = shallowMount(PapaParseAlert, { + propsData: { + papaParseErrors: errorMessages, + }, + }); + }; + + const findAlert = () => wrapper.findComponent(GlAlert); + + afterEach(() => { + wrapper.destroy(); + }); + + it('should render alert with correct props', async () => { + createComponent({ errorMessages: [{ code: 'MissingQuotes' }] }); + await nextTick; + + expect(findAlert().props()).toMatchObject({ + variant: 'danger', + }); + expect(findAlert().text()).toContain( + 'Failed to render the CSV file for the following reasons:', + ); + expect(findAlert().text()).toContain('Quoted field unterminated'); + }); + + it('should render original message if no translation available', async () => { + createComponent({ + errorMessages: [{ code: 'NotDefined', message: 'Error code is undefined' }], + }); + await nextTick; + + expect(findAlert().text()).toContain('Error code is undefined'); + }); +}); -- GitLab