diff --git a/changelog/entries/unreleased/bug/3028_builder_prevent_excessive_api_requests_in_record_selector.json b/changelog/entries/unreleased/bug/3028_builder_prevent_excessive_api_requests_in_record_selector.json new file mode 100644 index 000000000..653d01fa8 --- /dev/null +++ b/changelog/entries/unreleased/bug/3028_builder_prevent_excessive_api_requests_in_record_selector.json @@ -0,0 +1,7 @@ +{ + "type": "bug", + "message": "[Builder] Prevent excessive API requests in collection elements", + "issue_number": 3028, + "bullet_points": [], + "created_at": "2024-10-03" +} diff --git a/web-frontend/modules/builder/components/elements/components/RecordSelectorElement.vue b/web-frontend/modules/builder/components/elements/components/RecordSelectorElement.vue index fff52c55c..927234798 100644 --- a/web-frontend/modules/builder/components/elements/components/RecordSelectorElement.vue +++ b/web-frontend/modules/builder/components/elements/components/RecordSelectorElement.vue @@ -309,7 +309,7 @@ export default { canFetch() { // We want to fetch data only if the dropdown have been opened at least once. // It's not necessary otherwise - return this.openedOnce + return this.openedOnce && this.contentFetchEnabled }, getErrorMessage() { return this.displayFormDataError ? this.$t('error.requiredField') : '' diff --git a/web-frontend/modules/builder/components/elements/components/RepeatElement.vue b/web-frontend/modules/builder/components/elements/components/RepeatElement.vue index 7134d676a..a9f1a2ffb 100644 --- a/web-frontend/modules/builder/components/elements/components/RepeatElement.vue +++ b/web-frontend/modules/builder/components/elements/components/RepeatElement.vue @@ -98,7 +98,7 @@ <ABButton v-if="hasMorePage && children.length > 0" :style="getStyleOverride('button')" - :disabled="contentLoading" + :disabled="contentLoading || !contentFetchEnabled" :loading="contentLoading" @click="loadMore()" > diff --git a/web-frontend/modules/builder/components/elements/components/TableElement.vue b/web-frontend/modules/builder/components/elements/components/TableElement.vue index 5d6980c3d..84eed947a 100644 --- a/web-frontend/modules/builder/components/elements/components/TableElement.vue +++ b/web-frontend/modules/builder/components/elements/components/TableElement.vue @@ -45,7 +45,7 @@ <ABButton v-if="hasMorePage" :style="getStyleOverride('button')" - :disabled="contentLoading" + :disabled="contentLoading || !contentFetchEnabled" :loading="contentLoading" @click="loadMore()" > diff --git a/web-frontend/modules/builder/mixins/collectionElement.js b/web-frontend/modules/builder/mixins/collectionElement.js index b9ebed0d9..0b1849d6c 100644 --- a/web-frontend/modules/builder/mixins/collectionElement.js +++ b/web-frontend/modules/builder/mixins/collectionElement.js @@ -9,6 +9,7 @@ export default { currentOffset: 0, errorNotified: false, resetTimeout: null, + contentFetchEnabled: true, } }, computed: { @@ -120,6 +121,9 @@ export default { }) this.currentOffset += this.element.items_per_page } catch (error) { + // Handle the HTTP error if needed + this.onContentFetchError(error) + // We need to only launch one toast error message per element, // not one per element fetch, or we can end up with many error // toasts per element sharing a datasource. @@ -137,7 +141,17 @@ export default { }, /** Overrides this if you want to prevent data fetching */ canFetch() { - return true + return this.contentFetchEnabled + }, + + /** Override this if you want to handle content fetch errors */ + onContentFetchError(error) { + // If the request failed without reaching the server, `error.response` + // will be `undefined`, so we need to check that before checking the + // HTTP status code + if (error.response && [400, 404].includes(error.response.status)) { + this.contentFetchEnabled = false + } }, }, } diff --git a/web-frontend/test/helpers/testApp.js b/web-frontend/test/helpers/testApp.js index 494aadfdf..e2fc55bff 100644 --- a/web-frontend/test/helpers/testApp.js +++ b/web-frontend/test/helpers/testApp.js @@ -7,6 +7,7 @@ import setupClient, { import setupDatabasePlugin from '@baserow/modules/database/plugin' import setupBuilderPlugin from '@baserow/modules/builder/plugin' +import setupIntegrationPlugin from '@baserow/modules/integrations/plugin' import { bootstrapVueContext } from '@baserow/test/helpers/components' import MockAdapter from 'axios-mock-adapter' import _ from 'lodash' @@ -40,6 +41,7 @@ function _createBaserowStoreAndRegistry(app, vueContext, extraPluginSetupFunc) { app, } setupDatabasePlugin(setupContext) + setupIntegrationPlugin(setupContext) setupBuilderPlugin(setupContext) setupHasFeaturePlugin(setupContext, (name, dep) => { app[`$${name}`] = dep diff --git a/web-frontend/test/unit/builder/components/elements/components/RecordSelectorElement.spec.js b/web-frontend/test/unit/builder/components/elements/components/RecordSelectorElement.spec.js new file mode 100644 index 000000000..8fa1bf4a9 --- /dev/null +++ b/web-frontend/test/unit/builder/components/elements/components/RecordSelectorElement.spec.js @@ -0,0 +1,119 @@ +import { TestApp } from '@baserow/test/helpers/testApp' +import RecordSelectorElement from '@baserow/modules/builder/components/elements/components/RecordSelectorElement.vue' +import flushPromises from 'flush-promises' + +// Ignore `notifyIf` and `notifyIf404` function calls +jest.mock('@baserow/modules/core/utils/error.js') + +describe('RecordSelectorElement', () => { + let testApp = null + let store = null + let mockServer = null + + beforeAll(() => { + // NOTE: TestApp wraps any exception raised by the axios mock adapter and + // re-raises it as a Jest error. + // This mutates the error object and make some properties not available. + // In this case `collectionElement` mixin needs to access the response + // object when the server returns a 400/404 error, so we disable + // `failTestOnErrorResponse`. + testApp = new TestApp() + testApp.failTestOnErrorResponse = false + store = testApp.store + mockServer = testApp.mockServer + }) + + afterEach(() => { + testApp.afterEach() + }) + + const mountComponent = ({ props = {}, slots = {}, provide = {} }) => { + return testApp.mount(RecordSelectorElement, { + propsData: props, + slots, + provide, + }) + } + + test('does not paginate if API returns 400/404', async () => { + const builder = { id: 1, theme: { primary_color: '#ccc' } } + const page = { + id: 1, + dataSources: [{ id: 1, type: 'local_baserow_list_rows', table_id: 1 }], + elements: [], + } + const workspace = {} + const mode = 'public' + const element = { + id: 1, + type: 'record_selector', + data_source_id: page.dataSources[0].id, + items_Per_page: 5, + } + store.dispatch('element/forceCreate', { page, element }) + + const wrapper = await mountComponent({ + props: { + element, + }, + provide: { + builder, + page, + mode, + applicationContext: { builder, page, mode }, + element, + workspace, + }, + }) + + // A mock server that mimics the data source dispatch endpoints. + // The first time it is called it returns a successful message, but the + // second time it returns a 400 response. + const url = `builder/domains/published/data-source/${page.dataSources[0].id}/dispatch/` + mockServer.mock + .onPost(url) + .replyOnce(200, { + results: [ + { id: 1, order: 1, name: 'First' }, + { id: 2, order: 1, name: 'Second' }, + { id: 3, order: 1, name: 'Third' }, + { id: 4, order: 1, name: 'Fourth' }, + { id: 5, order: 1, name: 'Fifth' }, + ], + has_next_page: true, + }) + .onPost(url) + .reply(400, { message: 'Bad Request' }) + + // The first time we trigger a next page, the server responds with 200 + // therefore we should be able to fetch more content + await wrapper + .findAllComponents({ name: 'ABDropdown' }) + .at(0) + .find('.ab-dropdown__selected') + .trigger('click') + await flushPromises() + expect(wrapper.element).toMatchSnapshot() + expect(mockServer.mock.history.post.length).toBe(1) + + // Then we trigger a few scroll events in the record selector element and + // confirm that the API is only called once + await wrapper + .findAllComponents({ name: 'ABDropdown' }) + .at(0) + .find('.select__items') + .trigger('scroll') + await flushPromises() + expect(wrapper.element).toMatchSnapshot() + expect(mockServer.mock.history.post.length).toBe(2) + + await wrapper + .findAllComponents({ name: 'ABDropdown' }) + .at(0) + .find('.select__items') + .trigger('scroll') + await flushPromises() + expect(wrapper.element).toMatchSnapshot() + expect(mockServer.mock.history.post.length).toBe(2) + }) +}) diff --git a/web-frontend/test/unit/builder/components/elements/components/__snapshots__/RecordSelectorElement.spec.js.snap b/web-frontend/test/unit/builder/components/elements/components/__snapshots__/RecordSelectorElement.spec.js.snap new file mode 100644 index 000000000..bd7c05ef0 --- /dev/null +++ b/web-frontend/test/unit/builder/components/elements/components/__snapshots__/RecordSelectorElement.spec.js.snap @@ -0,0 +1,643 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`RecordSelectorElement does not paginate if API returns 400/404 1`] = ` +<div + class="control ab-form-group" +> + <!----> + + <!----> + + <div + class="control__wrapper" + > + <div + class="control__elements" + > + <div + class="flex-grow-1" + > + <!----> + + <div + class="ab-form-group__children" + > + <div + class="ab-dropdown choice-element" + tabindex="0" + > + <a + class="ab-dropdown__selected" + > + <!----> + + <span + class="ab-dropdown__selected-placeholder" + > + action.makeChoice + </span> + + <i + class="ab-dropdown__toggle-icon iconoir-nav-arrow-down" + /> + </a> + + <div + class="ab-dropdown__items" + > + <!----> + + <ul + class="select__items" + tabindex="-1" + > + + <section + class="infinite-scroll" + > + <li + class="ab-dropdownitem__item ab-dropdownitem__item--no-options" + > + <a + class="ab-dropdownitem__item-link" + > + <div + class="ab-dropdownitem__item-name" + > + <!----> + + <span + class="ab-dropdownitem__item-name-text" + title="First" + > + First + </span> + </div> + + <!----> + </a> + + <i + class="ab-dropdownitem__item-active-icon iconoir-check" + /> + </li> + <li + class="ab-dropdownitem__item ab-dropdownitem__item--no-options" + > + <a + class="ab-dropdownitem__item-link" + > + <div + class="ab-dropdownitem__item-name" + > + <!----> + + <span + class="ab-dropdownitem__item-name-text" + title="Second" + > + Second + </span> + </div> + + <!----> + </a> + + <i + class="ab-dropdownitem__item-active-icon iconoir-check" + /> + </li> + <li + class="ab-dropdownitem__item ab-dropdownitem__item--no-options" + > + <a + class="ab-dropdownitem__item-link" + > + <div + class="ab-dropdownitem__item-name" + > + <!----> + + <span + class="ab-dropdownitem__item-name-text" + title="Third" + > + Third + </span> + </div> + + <!----> + </a> + + <i + class="ab-dropdownitem__item-active-icon iconoir-check" + /> + </li> + <li + class="ab-dropdownitem__item ab-dropdownitem__item--no-options" + > + <a + class="ab-dropdownitem__item-link" + > + <div + class="ab-dropdownitem__item-name" + > + <!----> + + <span + class="ab-dropdownitem__item-name-text" + title="Fourth" + > + Fourth + </span> + </div> + + <!----> + </a> + + <i + class="ab-dropdownitem__item-active-icon iconoir-check" + /> + </li> + <li + class="ab-dropdownitem__item ab-dropdownitem__item--no-options" + > + <a + class="ab-dropdownitem__item-link" + > + <div + class="ab-dropdownitem__item-name" + > + <!----> + + <span + class="ab-dropdownitem__item-name-text" + title="Fifth" + > + Fifth + </span> + </div> + + <!----> + </a> + + <i + class="ab-dropdownitem__item-active-icon iconoir-check" + /> + </li> + + <div + class="infinite-scroll__loading-wrapper" + style="" + > + <!----> + </div> + + <!----> + </section> + </ul> + + <!----> + + <!----> + </div> + </div> + + <!----> + </div> + </div> + + </div> + + <!----> + </div> +</div> +`; + +exports[`RecordSelectorElement does not paginate if API returns 400/404 2`] = ` +<div + class="control ab-form-group" +> + <!----> + + <!----> + + <div + class="control__wrapper" + > + <div + class="control__elements" + > + <div + class="flex-grow-1" + > + <!----> + + <div + class="ab-form-group__children" + > + <div + class="ab-dropdown choice-element" + tabindex="0" + > + <a + class="ab-dropdown__selected" + > + <!----> + + <span + class="ab-dropdown__selected-placeholder" + > + action.makeChoice + </span> + + <i + class="ab-dropdown__toggle-icon iconoir-nav-arrow-down" + /> + </a> + + <div + class="ab-dropdown__items" + > + <!----> + + <ul + class="select__items" + tabindex="-1" + > + + <section + class="infinite-scroll" + > + <li + class="ab-dropdownitem__item ab-dropdownitem__item--no-options" + > + <a + class="ab-dropdownitem__item-link" + > + <div + class="ab-dropdownitem__item-name" + > + <!----> + + <span + class="ab-dropdownitem__item-name-text" + title="First" + > + First + </span> + </div> + + <!----> + </a> + + <i + class="ab-dropdownitem__item-active-icon iconoir-check" + /> + </li> + <li + class="ab-dropdownitem__item ab-dropdownitem__item--no-options" + > + <a + class="ab-dropdownitem__item-link" + > + <div + class="ab-dropdownitem__item-name" + > + <!----> + + <span + class="ab-dropdownitem__item-name-text" + title="Second" + > + Second + </span> + </div> + + <!----> + </a> + + <i + class="ab-dropdownitem__item-active-icon iconoir-check" + /> + </li> + <li + class="ab-dropdownitem__item ab-dropdownitem__item--no-options" + > + <a + class="ab-dropdownitem__item-link" + > + <div + class="ab-dropdownitem__item-name" + > + <!----> + + <span + class="ab-dropdownitem__item-name-text" + title="Third" + > + Third + </span> + </div> + + <!----> + </a> + + <i + class="ab-dropdownitem__item-active-icon iconoir-check" + /> + </li> + <li + class="ab-dropdownitem__item ab-dropdownitem__item--no-options" + > + <a + class="ab-dropdownitem__item-link" + > + <div + class="ab-dropdownitem__item-name" + > + <!----> + + <span + class="ab-dropdownitem__item-name-text" + title="Fourth" + > + Fourth + </span> + </div> + + <!----> + </a> + + <i + class="ab-dropdownitem__item-active-icon iconoir-check" + /> + </li> + <li + class="ab-dropdownitem__item ab-dropdownitem__item--no-options" + > + <a + class="ab-dropdownitem__item-link" + > + <div + class="ab-dropdownitem__item-name" + > + <!----> + + <span + class="ab-dropdownitem__item-name-text" + title="Fifth" + > + Fifth + </span> + </div> + + <!----> + </a> + + <i + class="ab-dropdownitem__item-active-icon iconoir-check" + /> + </li> + + <div + class="infinite-scroll__loading-wrapper" + style="" + > + <!----> + </div> + + <!----> + </section> + </ul> + + <!----> + + <!----> + </div> + </div> + + <!----> + </div> + </div> + + </div> + + <!----> + </div> +</div> +`; + +exports[`RecordSelectorElement does not paginate if API returns 400/404 3`] = ` +<div + class="control ab-form-group" +> + <!----> + + <!----> + + <div + class="control__wrapper" + > + <div + class="control__elements" + > + <div + class="flex-grow-1" + > + <!----> + + <div + class="ab-form-group__children" + > + <div + class="ab-dropdown choice-element" + tabindex="0" + > + <a + class="ab-dropdown__selected" + > + <!----> + + <span + class="ab-dropdown__selected-placeholder" + > + action.makeChoice + </span> + + <i + class="ab-dropdown__toggle-icon iconoir-nav-arrow-down" + /> + </a> + + <div + class="ab-dropdown__items" + > + <!----> + + <ul + class="select__items" + tabindex="-1" + > + + <section + class="infinite-scroll" + > + <li + class="ab-dropdownitem__item ab-dropdownitem__item--no-options" + > + <a + class="ab-dropdownitem__item-link" + > + <div + class="ab-dropdownitem__item-name" + > + <!----> + + <span + class="ab-dropdownitem__item-name-text" + title="First" + > + First + </span> + </div> + + <!----> + </a> + + <i + class="ab-dropdownitem__item-active-icon iconoir-check" + /> + </li> + <li + class="ab-dropdownitem__item ab-dropdownitem__item--no-options" + > + <a + class="ab-dropdownitem__item-link" + > + <div + class="ab-dropdownitem__item-name" + > + <!----> + + <span + class="ab-dropdownitem__item-name-text" + title="Second" + > + Second + </span> + </div> + + <!----> + </a> + + <i + class="ab-dropdownitem__item-active-icon iconoir-check" + /> + </li> + <li + class="ab-dropdownitem__item ab-dropdownitem__item--no-options" + > + <a + class="ab-dropdownitem__item-link" + > + <div + class="ab-dropdownitem__item-name" + > + <!----> + + <span + class="ab-dropdownitem__item-name-text" + title="Third" + > + Third + </span> + </div> + + <!----> + </a> + + <i + class="ab-dropdownitem__item-active-icon iconoir-check" + /> + </li> + <li + class="ab-dropdownitem__item ab-dropdownitem__item--no-options" + > + <a + class="ab-dropdownitem__item-link" + > + <div + class="ab-dropdownitem__item-name" + > + <!----> + + <span + class="ab-dropdownitem__item-name-text" + title="Fourth" + > + Fourth + </span> + </div> + + <!----> + </a> + + <i + class="ab-dropdownitem__item-active-icon iconoir-check" + /> + </li> + <li + class="ab-dropdownitem__item ab-dropdownitem__item--no-options" + > + <a + class="ab-dropdownitem__item-link" + > + <div + class="ab-dropdownitem__item-name" + > + <!----> + + <span + class="ab-dropdownitem__item-name-text" + title="Fifth" + > + Fifth + </span> + </div> + + <!----> + </a> + + <i + class="ab-dropdownitem__item-active-icon iconoir-check" + /> + </li> + + <div + class="infinite-scroll__loading-wrapper" + style="" + > + <!----> + </div> + + <!----> + </section> + </ul> + + <!----> + + <!----> + </div> + </div> + + <!----> + </div> + </div> + + </div> + + <!----> + </div> +</div> +`;