Resolve "Filters get applied when row modal opens"

changelog/entries/unreleased/bug/1765_filters_get_applied_when_row_modal_opens.json

@@ -0,0 +1,7 @@
+{
+    "type": "bug",
+    "message": "Prevent filters to be applied when the user open the row edit modal.",
+    "issue_number": 1765,
+    "bullet_points": [],
+    "created_at": "2023-09-18"
+}

premium/web-frontend/modules/baserow_premium/components/views/calendar/CalendarView.vue

@@ -8,7 +8,7 @@
       :database="database"
       :table="table"
       :view="view"
-      @edit-row="openRowEditModal($event.id)"
+      @edit-row="openRowEditModal($event)"
       @create-row="openCreateRowModal"
     ></CalendarMonth>
     <RowCreateModal
@@ -189,9 +189,9 @@ export default {
      * the Table component that a new row has been selected,
      * such that we can update the path to include the row id.
      */
-    openRowEditModal(rowId) {
-      this.$refs.rowEditModal.show(rowId)
-      this.$emit('selected-row', rowId)
+    openRowEditModal(row) {
+      this.$refs.rowEditModal.show(row.id)
+      this.$emit('selected-row', row)
     },
     /**
      * Populates a new row and opens the row edit modal
@@ -226,6 +226,10 @@ export default {
     openCreateRowModal(event) {
       const defaults = {}
       const dateField = this.getDateField(this.fields)
+      if (!dateField) {
+        // Cannot create a row without a proper date field
+        return
+      }
       const fieldType = this.$registry.get('field', dateField.type)
       if (event?.day?.date != null && dateField && !fieldType.getIsReadOnly()) {
         const name = `field_${dateField.id}`

premium/web-frontend/modules/baserow_premium/components/views/kanban/KanbanView.vue

@@ -32,7 +32,7 @@
         :read-only="readOnly"
         :store-prefix="storePrefix"
         @create-row="openCreateRowModal"
-        @edit-row="openRowEditModal($event.id)"
+        @edit-row="openRowEditModal($event)"
         @refresh="$emit('refresh', $event)"
       ></KanbanViewStack>
       <KanbanViewStack
@@ -47,7 +47,7 @@
         :read-only="readOnly"
         :store-prefix="storePrefix"
         @create-row="openCreateRowModal"
-        @edit-row="openRowEditModal($event.id)"
+        @edit-row="openRowEditModal($event)"
         @refresh="$emit('refresh', $event)"
       ></KanbanViewStack>
       <a

web-frontend/modules/database/components/view/gallery/GalleryView.vue

@@ -452,7 +452,7 @@ export default {
      */
     rowClick(row) {
       this.$refs.rowEditModal.show(row.id)
-      this.$emit('selected-row', row.id)
+      this.$emit('selected-row', row)
     },
     /**
      * Calls the fieldCreated callback and shows the hidden fields section

web-frontend/modules/database/components/view/grid/GridView.vue

@@ -51,7 +51,7 @@
       @selected="selectedCell"
       @unselected="unselectedCell"
       @select-next="selectNextCell"
-      @edit-modal="openRowEditModal($event.id)"
+      @edit-modal="openRowEditModal($event)"
       @scroll="scroll($event.pixelY, 0)"
     >
       <template #foot>
@@ -118,7 +118,7 @@
       @selected="selectedCell"
       @unselected="unselectedCell"
       @select-next="selectNextCell"
-      @edit-modal="openRowEditModal($event.id)"
+      @edit-modal="openRowEditModal($event)"
       @scroll="scroll($event.pixelY, $event.pixelX)"
     >
     </GridViewSection>
@@ -223,11 +223,7 @@
           </a>
         </li>
         <li>
-          <a
-            @click="
-              ;[openRowEditModal(selectedRow.id), $refs.rowContext.hide()]
-            "
-          >
+          <a @click=";[openRowEditModal(selectedRow), $refs.rowContext.hide()]">
             <i class="context__menu-icon iconoir-expand"></i>
             {{ $t('gridView.enlargeRow') }}
           </a>
@@ -433,9 +429,23 @@ export default {
     },
     row: {
       deep: true,
-      handler(row) {
-        if (row !== null && this.$refs.rowEditModal) {
-          this.populateAndEditRow(row)
+      handler(newRow, prevRow) {
+        if (newRow !== null && this.$refs.rowEditModal) {
+          this.populateAndEditRow(newRow)
+        }
+        // `refreshRow` doesn't immediately hide a row not matching filters if a
+        // user open the modal for that row to solve
+        // https://gitlab.com/baserow/baserow/-/issues/1765. This handler ensure
+        // the row is correctly refreshed if the user open another row using the
+        // navigation buttons in the modal.
+        const prevRowId = prevRow?.id
+        if (prevRowId !== undefined && prevRowId !== newRow?.id) {
+          this.$store.dispatch(this.storePrefix + 'view/grid/refreshRowById', {
+            grid: this.view,
+            fields: this.fields,
+            rowId: prevRowId,
+            getScrollTop: () => this.$refs.left.$refs.body.scrollTop,
+          })
         }
       },
     },
@@ -871,21 +881,21 @@ export default {
         return
       }
 
-      this.$store.dispatch(this.storePrefix + 'view/grid/refreshRow', {
+      this.$store.dispatch(this.storePrefix + 'view/grid/refreshRowById', {
         grid: this.view,
         fields: this.fields,
-        row,
+        rowId: row.id,
         getScrollTop: () => this.$refs.left.$refs.body.scrollTop,
       })
     },
     /**
-     * When the row edit modal is opened we notifiy
+     * When the row edit modal is opened we notify
      * the Table component that a new row has been selected,
      * such that we can update the path to include the row id.
      */
-    openRowEditModal(rowId) {
-      this.$refs.rowEditModal.show(rowId)
-      this.$emit('selected-row', rowId)
+    openRowEditModal(row) {
+      this.$refs.rowEditModal.show(row.id)
+      this.$emit('selected-row', row)
     },
     /**
      * Populates a new row and opens the row edit modal
@@ -951,6 +961,7 @@ export default {
             row,
             field,
             getScrollTop,
+            isRowOpenedInModal: this.rowOpenedInModal?.id === row.id,
           }
         )
       })

web-frontend/modules/database/pages/table.vue

@@ -42,6 +42,14 @@ export default {
     this.$store.dispatch('application/unselect')
     next()
   },
+  /**
+   * If a `rowId` is provided in the route params, we want to immediately open
+   * the row modal in the table page and show the `database-table-row` URL in
+   * the browser. This function parses the params and fetches the data needed to
+   * render the page correctly, redirecting to the table page if the row is not
+   * found. If the row is found in the store or in the backend, calling `next()`
+   * will open the row modal and will update the URL in the browser correctly.
+   */
   async beforeRouteUpdate(to, from, next) {
     function parseIntOrNull(x) {
       return x != null ? parseInt(x) : null
@@ -50,15 +58,20 @@ export default {
     const currentTableId = parseIntOrNull(to.params.tableId)
 
     const storeRow = this.$store.getters['rowModalNavigation/getRow']
+    const prevTableId = parseIntOrNull(from.params.tableId)
     const failedToFetchTableRowId =
       this.$store.getters['rowModalNavigation/getFailedToFetchTableRowId']
+
     if (currentRowId == null) {
+      // If the rowId is null, we want to close the row modal and show the table
+      // page, so clear the store accordingly.
       await this.$store.dispatch('rowModalNavigation/clearRow')
     } else if (
       failedToFetchTableRowId &&
       parseIntOrNull(failedToFetchTableRowId?.rowId) === currentRowId &&
       parseIntOrNull(failedToFetchTableRowId?.tableId) === currentTableId
     ) {
+      // Show the table page if the row failed to fetch.
       return next({
         name: 'database-table',
         params: {
@@ -68,8 +81,11 @@ export default {
       })
     } else if (
       storeRow?.id !== currentRowId ||
-      storeRow?.table_id !== currentTableId
+      prevTableId !== currentTableId
     ) {
+      // Fetch the row if it's not already in the store. If the row is not found,
+      // the store will be updated with the failedToFetchTableRowId and the table
+      // page will be shown.
       const row = await this.$store.dispatch('rowModalNavigation/fetchRow', {
         tableId: currentTableId,
         rowId: currentRowId,
@@ -243,8 +259,7 @@ export default {
     },
     async setAdjacentRow(previous, row = null, activeSearchTerm = null) {
       if (row) {
-        await this.$store.dispatch('rowModalNavigation/setRow', row)
-        this.navigateToRowModal(row.id)
+        await this.navigateToRowModal(row)
       } else {
         // If the row isn't provided then the row is
         // probably not visible to the user at the moment
@@ -252,10 +267,8 @@ export default {
         await this.fetchAdjacentRow(previous, activeSearchTerm)
       }
     },
-    selectRow(rowId) {
-      this.navigateToRowModal(rowId)
-    },
-    navigateToRowModal(rowId) {
+    async navigateToRowModal(row) {
+      const rowId = row?.id
       if (
         this.$route.params.rowId !== undefined &&
         this.$route.params.rowId === rowId
@@ -263,6 +276,12 @@ export default {
         return
       }
 
+      if (row) {
+        // Prevent the row from being fetched again from the backend
+        // when the route is updated
+        await this.$store.dispatch('rowModalNavigation/setRow', row)
+      }
+
       const location = {
         name: rowId ? 'database-table-row' : 'database-table',
         params: {
@@ -301,7 +320,7 @@ export default {
       }
 
       if (row) {
-        this.navigateToRowModal(row.id)
+        await this.navigateToRowModal(row)
       }
     },
   },

web-frontend/modules/database/store/rowModalNavigation.js

@@ -37,6 +37,7 @@ export const mutations = {
     state.row = row
   },
   SET_FAILED_TO_FETCH_TABLE_ROW_ID(state, tableAndRowId) {
+    state.row = null
     state.failedToFetchTableRowId = tableAndRowId
   },
 }

web-frontend/modules/database/store/view/grid.js

@@ -397,6 +397,9 @@ export const mutations = {
       }
     }
   },
+  UPDATE_ROW_VALUES(state, { row, values }) {
+    Object.assign(row, values)
+  },
   UPDATE_ROW_FIELD_VALUE(state, { row, field, value }) {
     row[`field_${field.id}`] = value
   },
@@ -1549,10 +1552,16 @@ export const actions = {
    */
   removeRowSelectedBy(
     { dispatch, commit },
-    { grid, row, field, fields, getScrollTop }
+    { grid, row, field, fields, getScrollTop, isRowOpenedInModal = false }
   ) {
     commit('REMOVE_ROW_SELECTED_BY', { row, fieldId: field.id })
-    dispatch('refreshRow', { grid, row, fields, getScrollTop })
+    dispatch('refreshRow', {
+      grid,
+      row,
+      fields,
+      getScrollTop,
+      isRowOpenedInModal,
+    })
   },
   /**
    * Called when the user wants to create a new row. Optionally a `before` row
@@ -1880,23 +1889,48 @@ export const actions = {
   },
   /**
    * Updates a grid view field value. It will immediately be updated in the store
-   * and only if the change request fails it will reverted to give a faster
+   * and only if the change request fails it will revert to give a faster
    * experience for the user.
    */
   async updateRowValue(
-    { commit, dispatch },
+    { commit, dispatch, getters },
     { table, view, row, field, fields, value, oldValue }
   ) {
-    // Immediately updated the store with the updated row field
-    // value.
-    commit('UPDATE_ROW_FIELD_VALUE', { row, field, value })
+    /**
+     * This helper function will make sure that the values of the related row are
+     * updated the right way.
+     */
+    const updateValues = async (values) => {
+      const rowExistsInBuffer = getters.getRow(row.id) !== undefined
+      if (rowExistsInBuffer) {
+        // If the row exists in the buffer, we can visually show to the user that
+        // the values have changed, without immediately reflecting the change in
+        // the buffer.
+        commit('UPDATE_ROW_VALUES', {
+          row,
+          values: { ...values },
+        })
+        await dispatch('onRowChange', { view, row, fields })
+      } else {
+        // If the row doesn't exist in the buffer, it could be that the new values
+        // bring in into there. Dispatching the `updatedExistingRow` will make
+        // sure that will happen in the right way.
+        await dispatch('updatedExistingRow', { view, fields, row, values })
+        await dispatch('fetchByScrollTopDelayed', {
+          scrollTop: getters.getScrollTop,
+          fields,
+        })
+      }
+    }
 
-    const optimisticFieldValues = {}
-    const valuesBeforeOptimisticUpdate = {}
-
-    // Store the before value of the field that gets updated
-    // in case we need to rollback changes
-    valuesBeforeOptimisticUpdate[`field_${field.id}`] = oldValue
+    const fieldValues = {
+      [`field_${field.id}`]: value,
+    }
+    const valuesBeforeUpdate = {
+      // Store the before value of the field that gets updated in case we need to
+      // rollback changes.
+      [`field_${field.id}`]: oldValue,
+    }
 
     let fieldsToCallOnRowChange = fields
 
@@ -1917,15 +1951,14 @@ export const actions = {
       )
 
       if (currentFieldValue !== optimisticFieldValue) {
-        optimisticFieldValues[fieldID] = optimisticFieldValue
-        valuesBeforeOptimisticUpdate[fieldID] = currentFieldValue
+        fieldValues[fieldID] = optimisticFieldValue
+        valuesBeforeUpdate[fieldID] = currentFieldValue
       }
     })
-    commit('UPDATE_ROW_IN_BUFFER', {
-      row,
-      values: { ...optimisticFieldValues },
-    })
-    dispatch('onRowChange', { view, row, fields })
+
+    // Update the values before making a request to the backend to make it feel
+    // instant for the user.
+    await updateValues(fieldValues)
 
     const fieldType = this.$registry.get('field', field._.type.type)
     const newValue = fieldType.prepareValueForUpdate(field, value)
@@ -1938,18 +1971,13 @@ export const actions = {
         row.id,
         values
       )
-      commit('UPDATE_ROW_IN_BUFFER', { row, values: updatedRow.data })
-      dispatch('onRowChange', { view, row, fields })
+      // Update the remaining values like formula, which depend on the backend.
+      await updateValues(updatedRow.data)
       dispatch('fetchAllFieldAggregationData', {
         view,
       })
     } catch (error) {
-      commit('UPDATE_ROW_IN_BUFFER', {
-        row,
-        values: { ...valuesBeforeOptimisticUpdate },
-      })
-
-      dispatch('onRowChange', { view, row, fields })
+      await updateValues(valuesBeforeUpdate)
       throw error
     }
   },
@@ -2533,16 +2561,43 @@ export const actions = {
 
     commit('SET_ROW_MATCH_SORTINGS', { row, value: currentIndex === newIndex })
   },
+  /**
+   * Refreshes the row in the store if the given rowId exists. If the row
+   * doesn't exist in the store, nothing will happen. This method ensures that
+   * the row refreshed is the one of this store, because it could be that the
+   * row object could come from another store.
+   */
+  async refreshRowById(
+    { dispatch, getters },
+    { grid, rowId, fields, getScrollTop, isRowOpenedInModal = false }
+  ) {
+    const row = getters.getRow(rowId)
+    if (row === undefined) {
+      return
+    }
+
+    await dispatch('refreshRow', {
+      grid,
+      row,
+      fields,
+      getScrollTop,
+      isRowOpenedInModal,
+    })
+  },
   /**
    * The row is going to be removed or repositioned if the matchFilters and
    * matchSortings state is false. It will make the state correct.
    */
   async refreshRow(
-    { dispatch, commit, getters },
-    { grid, row, fields, getScrollTop }
+    { dispatch, commit },
+    { grid, row, fields, getScrollTop, isRowOpenedInModal = false }
   ) {
     const rowShouldBeHidden = !row._.matchFilters || !row._.matchSearch
-    if (row._.selectedBy.length === 0 && rowShouldBeHidden) {
+    if (
+      row._.selectedBy.length === 0 &&
+      rowShouldBeHidden &&
+      !isRowOpenedInModal
+    ) {
       commit('DELETE_ROW_IN_BUFFER', row)
     } else if (row._.selectedBy.length === 0 && !row._.matchSortings) {
       await dispatch('updatedExistingRow', {