Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"type": "bug",
"message": "Fixes hourly and daily periodic data syncs silently skipping scheduled runs",
"issue_origin": "github",
"issue_number": null,
"domain": "database",
"bullet_points": [],
"created_at": "2026-07-10"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"type": "bug",
"message": "Improved element internal page navigation by requiring page parameter values.",
"issue_origin": "github",
"issue_number": 3110,
"domain": "builder",
"bullet_points": [],
"created_at": "2026-07-03"
}
39 changes: 22 additions & 17 deletions enterprise/backend/src/baserow_enterprise/data_sync/handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,27 +97,30 @@ def call_periodic_data_sync_syncs_that_are_due(cls):
)

is_null = Q(last_periodic_sync__isnull=True)
daily_due = Q(
# If the interval is daily, the last periodic sync timestamp must be
# yesterday or None meaning it hasn't been executed yet.
is_null | Q(last_periodic_sync__lt=beginning_of_day),
interval=DATA_SYNC_INTERVAL_DAILY,
# The data sync must be triggered at the time desired by the user.
when__lte=now_time,
)
hourly_due = Q(
# If the interval is hourly, the last periodic data sync timestamp
# must be at least an hour ago or None meaning it hasn't been
# executed yet.
is_null | Q(last_periodic_sync__lt=beginning_of_hour),
# Only the minute and second of `when` matter because it runs every hour.
Q(when__minute__lt=now.minute)
| Q(when__minute=now.minute, when__second__lte=now.second),
interval=DATA_SYNC_INTERVAL_HOURLY,
)
all_to_trigger = (
PeriodicDataSyncInterval.objects.filter(
Q(
# If the interval is daily, the last periodic sync timestamp must be
# yesterday or None meaning it hasn't been executed yet.
is_null | Q(last_periodic_sync__lt=beginning_of_day),
interval=DATA_SYNC_INTERVAL_DAILY,
)
| Q(
# If the interval is hourly, the last periodic data sync timestamp
# must be at least an hour ago or None meaning it hasn't been
# executed yet.
is_null | Q(last_periodic_sync__lt=beginning_of_hour),
interval=DATA_SYNC_INTERVAL_HOURLY,
),
daily_due | hourly_due,
# Skip deactivated periodic data sync because they're not working
# anymore.
automatically_deactivated=False,
# The now time must be higher than the now time because the data sync
# must be triggered at the desired the of the user.
when__lte=now_time,
)
.select_related("data_sync__table__database__workspace")
# Take a lock on the periodic data sync because the `last_periodic_sync`
Expand Down Expand Up @@ -163,7 +166,9 @@ def call_periodic_data_sync_syncs_that_are_due(cls):
)
else:
transaction.on_commit(
lambda: sync_periodic_data_sync.delay(periodic_data_sync.id)
lambda pds=periodic_data_sync: sync_periodic_data_sync.delay(
pds.id
)
)
else:
periodic_data_sync.interval = DATA_SYNC_INTERVAL_MANUAL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,76 @@ def test_call_periodic_data_sync_syncs_starts_task(
assert args[0][0] == not_yet_executed_1.id


@pytest.mark.django_db(transaction=True)
@pytest.mark.data_sync
@override_settings(DEBUG=True)
@patch("baserow_enterprise.data_sync.handler.sync_periodic_data_sync")
def test_call_periodic_data_sync_syncs_starts_task_for_each_due_sync(
mock_sync_periodic_data_sync,
enterprise_data_fixture,
synced_roles,
):
enterprise_data_fixture.enable_enterprise()
user = enterprise_data_fixture.create_user()

due = [
EnterpriseDataSyncHandler.update_periodic_data_sync_interval(
user=user,
data_sync=enterprise_data_fixture.create_ical_data_sync(user=user),
interval="HOURLY",
when=time(hour=12, minute=10, second=1, microsecond=1),
)
for _ in range(3)
]

with freeze_time("2024-10-10T12:15:00.00Z"):
with transaction.atomic():
EnterpriseDataSyncHandler.call_periodic_data_sync_syncs_that_are_due()

called_ids = {
call.args[0] for call in mock_sync_periodic_data_sync.delay.call_args_list
}
assert called_ids == {periodic_data_sync.id for periodic_data_sync in due}


@pytest.mark.django_db
@pytest.mark.data_sync
@override_settings(DEBUG=True)
def test_call_hourly_periodic_data_sync_ignores_hour_of_when(enterprise_data_fixture):
enterprise_data_fixture.enable_enterprise()
user = enterprise_data_fixture.create_user()

minute_already_passed = (
EnterpriseDataSyncHandler.update_periodic_data_sync_interval(
user=user,
data_sync=enterprise_data_fixture.create_ical_data_sync(user=user),
interval="HOURLY",
when=time(hour=15, minute=10, second=1, microsecond=1),
)
)

minute_not_yet_passed = (
EnterpriseDataSyncHandler.update_periodic_data_sync_interval(
user=user,
data_sync=enterprise_data_fixture.create_ical_data_sync(user=user),
interval="HOURLY",
when=time(hour=8, minute=45, second=0),
)
)

with freeze_time("2024-10-10T10:30:00.00Z"):
EnterpriseDataSyncHandler.call_periodic_data_sync_syncs_that_are_due()
frozen_datetime = django_timezone.now()

minute_already_passed.refresh_from_db()
# executed because minute 10 has passed, despite the later hour in `when`.
assert minute_already_passed.last_periodic_sync == frozen_datetime

minute_not_yet_passed.refresh_from_db()
# not executed because minute 45 hasn't passed, despite the earlier hour in `when`.
assert minute_not_yet_passed.last_periodic_sync is None


@pytest.mark.django_db
@pytest.mark.data_sync
@override_settings(DEBUG=True)
Expand Down
5 changes: 4 additions & 1 deletion web-frontend/modules/builder/collectionFieldTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,10 @@ export class LinkCollectionFieldType extends CollectionFieldType {
) {
return this.app.$i18n.t('elementType.errorPageParameterInError')
}
} else if (field.navigation_type === 'custom' && !field.navigate_to_url) {
} else if (
field.navigation_type === 'custom' &&
!field.navigate_to_url.formula
) {
return this.app.$i18n.t('elementType.errorNavigationUrlMissing')
}

Expand Down
2 changes: 1 addition & 1 deletion web-frontend/modules/builder/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@
"errorIframeUrlMissing": "Missing IFrame URL property",
"errorIframeContentMissing": "Missing IFrame content",
"errorNoMenuItem": "No menu item configured",
"errorMenuItemInError": "At least one menu is misconfigured",
"errorMenuItemInError": "At least one menu item is misconfigured",
"errorSubMenuItemInError": "At least one sub menu is misconfigured"
},
"addElementButton": {
Expand Down
79 changes: 56 additions & 23 deletions web-frontend/modules/builder/utils/params.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,7 @@ export function defaultValueForParameterType(type) {
// This needs to match QUERY_PARAM_EXACT_MATCH_REGEX from backend.
export const QUERY_PARAM_REGEX = /^([A-Za-z][A-Za-z0-9_-]*)$/g

/**
* Responsible for detecting if a navigable record's path parameters have diverged
* from the destination page's path parameters. This can happen if a record
* points to a page, and then the page's parameters are altered.
*
* @param {Object} navigationObject - An `element` or `workflowAction` object
* which points to navigation data. In the case of an `element` this could be
* a button, and in the case of a `workflowAction` this could be an "open page"
* workflow action type.
* @param {Array} pages - An array of "visible" pages in the application.
* @returns {Boolean} Whether this navigable object has parameters in error.
*/
export function pathParametersInError(navigationObject, pages) {
function getNavigationPathParameters(navigationObject, pages) {
if (
navigationObject.navigation_type === 'page' &&
!isNaN(navigationObject.navigate_to_page_id)
Expand All @@ -37,19 +25,64 @@ export function pathParametersInError(navigationObject, pages) {
)

if (destinationPage) {
const destinationPageParams = destinationPage.path_params || []
const pageParams = navigationObject.page_parameters || []

const destinationPageParamNames = destinationPageParams.map(
({ name }) => name
)
const pageParamNames = pageParams.map(({ name }) => name)

if (!_.isEqual(destinationPageParamNames, pageParamNames)) {
return true
return {
destinationPageParams: destinationPage.path_params || [],
pageParams: navigationObject.page_parameters || [],
}
}
}
return null
}

function pathParameterHasValue(pageParam) {
const value = pageParam?.value
if (value === null || typeof value === 'undefined') {
return false
}
if (typeof value === 'object') {
return Boolean(value.formula)
}
return Boolean(value)
}

function pathParameterNamesMismatch(destinationPageParams, pageParams) {
const destinationPageParamNames = destinationPageParams.map(
({ name }) => name
)
const pageParamNames = pageParams.map(({ name }) => name)

return !_.isEqual(destinationPageParamNames, pageParamNames)
}

function pathParameterValuesMissing(destinationPageParams, pageParams) {
return destinationPageParams.some(({ name }) => {
const pageParam = pageParams.find((param) => param.name === name)
return !pathParameterHasValue(pageParam)
})
}

/**
* Responsible for detecting if a navigable record's path parameters are
* misconfigured. Parameters are in error when their saved names no longer match
* the destination page's path parameters, or when a required value is missing.
*
* @param {Object} navigationObject - An `element` or `workflowAction` object
* which points to navigation data.
* @param {Array} pages - An array of "visible" pages in the application.
* @returns {Boolean} Whether this navigable object has parameters in error.
*/
export function pathParametersInError(navigationObject, pages) {
const pathParameters = getNavigationPathParameters(navigationObject, pages)

if (pathParameters) {
const { destinationPageParams, pageParams } = pathParameters

return (
pathParameterNamesMismatch(destinationPageParams, pageParams) ||
pathParameterValuesMissing(destinationPageParams, pageParams)
)
}

return false
}

Expand Down
14 changes: 5 additions & 9 deletions web-frontend/modules/builder/workflowActionTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,19 +92,15 @@ export class OpenPageWorkflowActionType extends WorkflowActionType {
'workflowActionTypes.errorNavigateToPageMissing'
)
}
if (
pathParametersInError(
workflowAction,
this.app.$store.getters['page/getVisiblePages'](
applicationContext.builder
)
)
) {
const visiblePages = this.app.$store.getters['page/getVisiblePages'](
applicationContext.builder
)
if (pathParametersInError(workflowAction, visiblePages)) {
return this.app.$i18n.t('workflowActionTypes.errorPageParameterInError')
}
} else if (
workflowAction.navigation_type === 'custom' &&
!workflowAction.navigate_to_url
!workflowAction.navigate_to_url.formula
) {
return this.app.$i18n.t('workflowActionTypes.errorNavigationUrlMissing')
}
Expand Down
92 changes: 92 additions & 0 deletions web-frontend/test/unit/builder/collectionFieldTypes.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import { TestApp } from '@baserow/test/helpers/testApp'

describe('Builder collection field types', () => {
let testApp = null

beforeEach(() => {
testApp = new TestApp()
})

afterEach(async () => {
await testApp.afterEach()
})

describe('LinkCollectionFieldType getErrorMessage', () => {
test('is in error when custom navigation URL is missing', () => {
const fieldType = testApp.getRegistry().get('collectionField', 'link')
const builder = { id: 1, pages: [] }
const field = {
type: 'link',
link_name: { formula: "'Foo'" },
navigation_type: 'custom',
navigate_to_url: { formula: '' },
}

expect(fieldType.isInError({ field, builder })).toBe(true)
expect(fieldType.getErrorMessage({ field, builder })).toBe(
'elementType.errorNavigationUrlMissing'
)

// Once a custom URL formula is provided, the field is no longer in error.
field.navigate_to_url = { formula: "'https://baserow.io'" }

expect(fieldType.isInError({ field, builder })).toBe(false)
})

test('is in error when page navigation has no destination page', () => {
const fieldType = testApp.getRegistry().get('collectionField', 'link')
const builder = { id: 1, pages: [] }
const field = {
type: 'link',
link_name: { formula: "'Foo'" },
navigation_type: 'page',
navigate_to_page_id: '',
}

expect(fieldType.isInError({ field, builder })).toBe(true)
expect(fieldType.getErrorMessage({ field, builder })).toBe(
'elementType.errorNavigateToPageMissing'
)
})

test('is in error when a required page parameter is empty', () => {
const fieldType = testApp.getRegistry().get('collectionField', 'link')
const builder = {
id: 1,
pages: [
{
id: 1,
shared: false,
order: 1,
path: '/',
path_params: [],
},
{
id: 2,
shared: false,
order: 2,
path: '/details/:id',
path_params: [{ name: 'id', type: 'numeric' }],
},
],
}
const field = {
type: 'link',
link_name: { formula: "'Foo'" },
navigation_type: 'page',
navigate_to_page_id: 2,
page_parameters: [{ name: 'id', value: { formula: '' } }],
}

expect(fieldType.isInError({ field, builder })).toBe(true)
expect(fieldType.getErrorMessage({ field, builder })).toBe(
'elementType.errorPageParameterInError'
)

// Once the page parameter has a value, the field is no longer in error.
field.page_parameters = [{ name: 'id', value: { formula: "'10'" } }]

expect(fieldType.isInError({ field, builder })).toBe(false)
})
})
})
Loading
Loading