1
0
Fork 0
mirror of https://gitlab.com/bramw/baserow.git synced 2025-04-26 05:37:13 +00:00

Merge branch 'prevent-view-update-with-wrong-data' into 'develop'

Prevent view from updated state with data from wrong view when pending request completes

Closes 

See merge request 
This commit is contained in:
Bram Wiepjes 2025-02-20 18:39:43 +00:00
commit b21e5fc47b
5 changed files with 89 additions and 8 deletions
changelog/entries/unreleased/bug
premium/web-frontend/modules/baserow_premium/store/view
web-frontend/modules/database/store/view

View file

@ -0,0 +1,7 @@
{
"type": "bug",
"message": "Prevent view from updated state with data from wrong view when pending request completes.",
"issue_number": 3398,
"bullet_points": [],
"created_at": "2025-02-18"
}

View file

@ -304,6 +304,7 @@ export const actions = {
{ commit, getters, rootGetters }, { commit, getters, rootGetters },
{ dateTime, fields, includeFieldOptions = false } { dateTime, fields, includeFieldOptions = false }
) { ) {
const calendarId = getters.getLastCalendarId
commit('SET_SELECTED_DATE', dateTime) commit('SET_SELECTED_DATE', dateTime)
const df = getters.getDateField(fields) const df = getters.getDateField(fields)
@ -335,6 +336,12 @@ export const actions = {
publicUrl: rootGetters['page/view/public/getIsPublic'], publicUrl: rootGetters['page/view/public/getIsPublic'],
publicAuthToken: rootGetters['page/view/public/getAuthToken'], publicAuthToken: rootGetters['page/view/public/getAuthToken'],
}) })
// Don't do anything if the calendarId does not match the current view calendarId
// because that probably means the user switched to another view or table, and
// the data that is returned here shouldn't do anything.
if (calendarId !== getters.getLastCalendarId) {
return
}
const lastRequest = dateTime.isSame(getters.getSelectedDate(fields)) const lastRequest = dateTime.isSame(getters.getSelectedDate(fields))
if (lastRequest) { if (lastRequest) {
Object.keys(data.rows).forEach((key) => { Object.keys(data.rows).forEach((key) => {
@ -390,6 +397,12 @@ export const actions = {
publicAuthToken: rootGetters['page/view/public/getAuthToken'], publicAuthToken: rootGetters['page/view/public/getAuthToken'],
filters, filters,
}) })
// Don't do anything if the calendarId does not match the current view calendarId
// because that probably means the user switched to another view or table, and
// the data that is returned here shouldn't do anything.
if (calendarId !== getters.getLastCalendarId) {
return
}
const newRows = data.rows[date].results const newRows = data.rows[date].results
const newCount = data.rows[date].count const newCount = data.rows[date].count
newRows.forEach((row) => { newRows.forEach((row) => {

View file

@ -211,6 +211,7 @@ export const actions = {
} }
) { ) {
commit('SET_ADHOC_FILTERING', adhocFiltering) commit('SET_ADHOC_FILTERING', adhocFiltering)
commit('SET_LAST_KANBAN_ID', kanbanId)
const view = rootGetters['view/get'](kanbanId) const view = rootGetters['view/get'](kanbanId)
const { data } = await KanbanService(this.$client).fetchRows({ const { data } = await KanbanService(this.$client).fetchRows({
kanbanId, kanbanId,
@ -222,10 +223,15 @@ export const actions = {
publicAuthToken: rootGetters['page/view/public/getAuthToken'], publicAuthToken: rootGetters['page/view/public/getAuthToken'],
filters: getFilters(view, adhocFiltering), filters: getFilters(view, adhocFiltering),
}) })
// Don't do anything if the kanbanId does not match the current view kanbanId
// because that probably means the user switched to another view or table, and
// the data that is returned here shouldn't do anything.
if (kanbanId !== getters.getLastKanbanId) {
return
}
Object.keys(data.rows).forEach((key) => { Object.keys(data.rows).forEach((key) => {
populateStack(data.rows[key], data) populateStack(data.rows[key], data)
}) })
commit('SET_LAST_KANBAN_ID', kanbanId)
commit('SET_SINGLE_SELECT_FIELD_ID', singleSelectFieldId) commit('SET_SINGLE_SELECT_FIELD_ID', singleSelectFieldId)
commit('REPLACE_ALL_STACKS', data.rows) commit('REPLACE_ALL_STACKS', data.rows)
if (includeFieldOptions) { if (includeFieldOptions) {
@ -241,10 +247,11 @@ export const actions = {
{ dispatch, commit, getters, rootGetters }, { dispatch, commit, getters, rootGetters },
{ selectOptionId } { selectOptionId }
) { ) {
const kanbanId = getters.getLastKanbanId
const stack = getters.getStack(selectOptionId) const stack = getters.getStack(selectOptionId)
const view = rootGetters['view/get'](getters.getLastKanbanId) const view = rootGetters['view/get'](kanbanId)
const { data } = await KanbanService(this.$client).fetchRows({ const { data } = await KanbanService(this.$client).fetchRows({
kanbanId: getters.getLastKanbanId, kanbanId,
limit: getters.getBufferRequestSize, limit: getters.getBufferRequestSize,
offset: 0, offset: 0,
includeFieldOptions: false, includeFieldOptions: false,
@ -259,6 +266,12 @@ export const actions = {
publicAuthToken: rootGetters['page/view/public/getAuthToken'], publicAuthToken: rootGetters['page/view/public/getAuthToken'],
filters: getFilters(view, getters.getAdhocFiltering), filters: getFilters(view, getters.getAdhocFiltering),
}) })
// Don't do anything if the kanbanId does not match the current view kanbanId
// because that probably means the user switched to another view or table, and
// the data that is returned here shouldn't do anything.
if (kanbanId !== getters.getLastKanbanId) {
return
}
const count = data.rows[selectOptionId].count const count = data.rows[selectOptionId].count
const rows = data.rows[selectOptionId].results const rows = data.rows[selectOptionId].results
rows.forEach((row) => { rows.forEach((row) => {

View file

@ -321,6 +321,12 @@ export default ({ service, customPopulateRow, fieldOptions }) => {
filters: getFilters(view, adhocFiltering), filters: getFilters(view, adhocFiltering),
...initialRowArguments, ...initialRowArguments,
}) })
// Don't do anything if the viewId does not match the current view viewId
// because that probably means the user switched to another view or table, and
// the data that is returned here shouldn't do anything.
if (viewId !== getters.getViewId) {
return
}
const rows = Array(data.count).fill(null) const rows = Array(data.count).fill(null)
data.results.forEach((row, index) => { data.results.forEach((row, index) => {
const metadata = extractRowMetadata(data, row.id) const metadata = extractRowMetadata(data, row.id)
@ -339,6 +345,7 @@ export default ({ service, customPopulateRow, fieldOptions }) => {
{ dispatch, getters, commit, rootGetters }, { dispatch, getters, commit, rootGetters },
parameters parameters
) { ) {
const viewId = getters.getViewId
const { startIndex, endIndex } = parameters const { startIndex, endIndex } = parameters
// If the store is already fetching a set of pages, we're temporarily storing // If the store is already fetching a set of pages, we're temporarily storing
@ -377,7 +384,7 @@ export default ({ service, customPopulateRow, fieldOptions }) => {
return return
} }
const view = rootGetters['view/get'](getters.getViewId) const view = rootGetters['view/get'](viewId)
// We can only make one request at the same time, so we're going to set the // We can only make one request at the same time, so we're going to set the
// fetching state to `true` to prevent multiple requests being fired // fetching state to `true` to prevent multiple requests being fired
@ -386,7 +393,7 @@ export default ({ service, customPopulateRow, fieldOptions }) => {
lastRequestController = new AbortController() lastRequestController = new AbortController()
try { try {
const { data } = await service(this.$client).fetchRows({ const { data } = await service(this.$client).fetchRows({
viewId: getters.getViewId, viewId,
offset: rangeToFetch.offset, offset: rangeToFetch.offset,
limit: rangeToFetch.limit, limit: rangeToFetch.limit,
signal: lastRequestController.signal, signal: lastRequestController.signal,
@ -397,6 +404,12 @@ export default ({ service, customPopulateRow, fieldOptions }) => {
orderBy: getOrderBy(view, getters.getAdhocSorting), orderBy: getOrderBy(view, getters.getAdhocSorting),
filters: getFilters(view, getters.getAdhocFiltering), filters: getFilters(view, getters.getAdhocFiltering),
}) })
// Don't do anything if the viewId does not match the current view viewId
// because that probably means the user switched to another view or table, and
// the data that is returned here shouldn't do anything.
if (viewId !== getters.getViewId) {
return
}
commit('UPDATE_ROWS', { commit('UPDATE_ROWS', {
offset: rangeToFetch.offset, offset: rangeToFetch.offset,
rows: data.results, rows: data.results,
@ -429,6 +442,7 @@ export default ({ service, customPopulateRow, fieldOptions }) => {
{ dispatch, commit, getters, rootGetters }, { dispatch, commit, getters, rootGetters },
{ fields, adhocFiltering, adhocSorting, includeFieldOptions = false } { fields, adhocFiltering, adhocSorting, includeFieldOptions = false }
) { ) {
const viewId = getters.getViewId
commit('SET_ADHOC_FILTERING', adhocFiltering) commit('SET_ADHOC_FILTERING', adhocFiltering)
commit('SET_ADHOC_SORTING', adhocSorting) commit('SET_ADHOC_SORTING', adhocSorting)
// If another refresh or fetch request is currently running, we need to cancel // If another refresh or fetch request is currently running, we need to cancel
@ -439,7 +453,7 @@ export default ({ service, customPopulateRow, fieldOptions }) => {
} }
lastRequestController = new AbortController() lastRequestController = new AbortController()
const view = rootGetters['view/get'](getters.getViewId) const view = rootGetters['view/get'](viewId)
try { try {
// We first need to fetch the count of all rows because we need to know how // We first need to fetch the count of all rows because we need to know how
// many rows there are in total to estimate what are new visible range it // many rows there are in total to estimate what are new visible range it
@ -448,7 +462,7 @@ export default ({ service, customPopulateRow, fieldOptions }) => {
const { const {
data: { count }, data: { count },
} = await service(this.$client).fetchCount({ } = await service(this.$client).fetchCount({
viewId: getters.getViewId, viewId,
signal: lastRequestController.signal, signal: lastRequestController.signal,
search: getters.getServerSearchTerm, search: getters.getServerSearchTerm,
searchMode: getDefaultSearchModeFromEnv(this.$config), searchMode: getDefaultSearchModeFromEnv(this.$config),
@ -487,7 +501,7 @@ export default ({ service, customPopulateRow, fieldOptions }) => {
// Only fetch visible rows if there are any. // Only fetch visible rows if there are any.
const { data } = await service(this.$client).fetchRows({ const { data } = await service(this.$client).fetchRows({
viewId: getters.getViewId, viewId,
offset: rangeToFetch.offset, offset: rangeToFetch.offset,
limit: rangeToFetch.limit, limit: rangeToFetch.limit,
includeFieldOptions, includeFieldOptions,
@ -500,6 +514,13 @@ export default ({ service, customPopulateRow, fieldOptions }) => {
filters: getFilters(view, adhocFiltering), filters: getFilters(view, adhocFiltering),
}) })
// Don't do anything if the viewId does not match the current view viewId
// because that probably means the user switched to another view or table, and
// the data that is returned here shouldn't do anything.
if (viewId !== getters.getViewId) {
return
}
data.results.forEach((row, index) => { data.results.forEach((row, index) => {
const metadata = extractRowMetadata(data, row.id) const metadata = extractRowMetadata(data, row.id)
rows[rangeToFetch.offset + index] = populateRow(row, metadata) rows[rangeToFetch.offset + index] = populateRow(row, metadata)

View file

@ -777,6 +777,13 @@ export const actions = {
filters: getFilters(view, getters.getAdhocFiltering), filters: getFilters(view, getters.getAdhocFiltering),
}) })
.then(({ data }) => { .then(({ data }) => {
// Don't do anything if the gridId does not match the current view gridId
// because that probably means the user switched to another view or table, and
// the data that is returned here shouldn't do anything.
if (gridId !== getters.getLastGridId) {
return
}
data.results.forEach((row) => { data.results.forEach((row) => {
const metadata = extractRowMetadata(data, row.id) const metadata = extractRowMetadata(data, row.id)
populateRow(row, metadata) populateRow(row, metadata)
@ -943,6 +950,12 @@ export const actions = {
orderBy: getOrderBy(view, adhocSorting), orderBy: getOrderBy(view, adhocSorting),
filters: getFilters(view, adhocFiltering), filters: getFilters(view, adhocFiltering),
}) })
// Don't do anything if the gridId does not match the current view gridId
// because that probably means the user switched to another view or table, and
// the data that is returned here shouldn't do anything.
if (gridId !== getters.getLastGridId) {
return
}
data.results.forEach((row) => { data.results.forEach((row) => {
const metadata = extractRowMetadata(data, row.id) const metadata = extractRowMetadata(data, row.id)
populateRow(row, metadata) populateRow(row, metadata)
@ -1028,6 +1041,12 @@ export const actions = {
})) }))
) )
.then(({ data, offset }) => { .then(({ data, offset }) => {
// Don't do anything if the gridId does not match the current view gridId
// because that probably means the user switched to another view or table, and
// the data that is returned here shouldn't do anything.
if (gridId !== getters.getLastGridId) {
return
}
// If there are results we can replace the existing rows so that the user stays // If there are results we can replace the existing rows so that the user stays
// at the same scroll offset. // at the same scroll offset.
data.results.forEach((row) => { data.results.forEach((row) => {
@ -1190,6 +1209,7 @@ export const actions = {
const isPublic = rootGetters['page/view/public/getIsPublic'] const isPublic = rootGetters['page/view/public/getIsPublic']
const search = getters.getActiveSearchTerm const search = getters.getActiveSearchTerm
const fieldOptions = getters.getAllFieldOptions const fieldOptions = getters.getAllFieldOptions
const gridId = getters.getLastGridId
let atLeastOneAggregation = false let atLeastOneAggregation = false
Object.entries(fieldOptions).forEach(([fieldId, options]) => { Object.entries(fieldOptions).forEach(([fieldId, options]) => {
@ -1239,6 +1259,13 @@ export const actions = {
const { data } = await lastAggregationRequest.request const { data } = await lastAggregationRequest.request
lastAggregationRequest.request = null lastAggregationRequest.request = null
// Don't do anything if the gridId does not match the current view gridId
// because that probably means the user switched to another view or table, and
// the data that is returned here shouldn't do anything.
if (gridId !== getters.getLastGridId) {
return
}
Object.entries(fieldOptions).forEach(([fieldId, options]) => { Object.entries(fieldOptions).forEach(([fieldId, options]) => {
if (options.aggregation_raw_type) { if (options.aggregation_raw_type) {
commit('SET_FIELD_AGGREGATION_DATA', { commit('SET_FIELD_AGGREGATION_DATA', {