From 1cf1ee0b8356714a42d51b09f063981c0d7e49e6 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Fri, 8 Sep 2023 12:13:09 +0200 Subject: [PATCH 1/8] fix(xo-web/_getScheduleJob): jobs can be undefined Related to #6968 --- packages/xo-web/src/xo-app/jobs/overview/index.js | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/packages/xo-web/src/xo-app/jobs/overview/index.js b/packages/xo-web/src/xo-app/jobs/overview/index.js index ffc2c746766..66156f4db37 100644 --- a/packages/xo-web/src/xo-app/jobs/overview/index.js +++ b/packages/xo-web/src/xo-app/jobs/overview/index.js @@ -110,6 +110,7 @@ export default class Overview extends Component { constructor(props) { super(props) this.state = { + jobs: {}, schedules: [], } } @@ -129,7 +130,7 @@ export default class Overview extends Component { const unsubscribeSchedules = subscribeSchedules(schedules => { // Get only backup jobs. schedules = filter(schedules, schedule => { - const job = this._getScheduleJob(schedule) + const job = this.state.jobs[schedule.jobId] return job && jobKeyToLabel[job.key] }) @@ -144,19 +145,15 @@ export default class Overview extends Component { } } - _getScheduleJob(schedule) { - const { jobs } = this.state || {} - return jobs[schedule.jobId] - } - _getIsScheduleUserMissing = createSelector( () => this.state.schedules, () => this.props.users, - (schedules, users) => { + () => this.state.jobs, + (schedules, users, jobs) => { const isScheduleUserMissing = {} forEach(schedules, schedule => { - isScheduleUserMissing[schedule.id] = !find(users, user => user.id === this._getScheduleJob(schedule).userId) + isScheduleUserMissing[schedule.id] = !find(users, user => user.id === jobs[schedule.jobId].userId) }) return isScheduleUserMissing @@ -197,7 +194,7 @@ export default class Overview extends Component { collection={schedules} columns={SCHEDULES_COLUMNS} data-isScheduleUserMissing={this._getIsScheduleUserMissing()} - data-jobs={this.state.jobs || {}} + data-jobs={this.state.jobs} individualActions={this._individualActions} shortcutsTarget='body' stateUrlParam='s' From 6635ba2eccb18c4b473a73f8f159de107bd2fb1b Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Tue, 12 Sep 2023 10:45:18 +0200 Subject: [PATCH 2/8] chore(xo-web/_getScheduleJob): fix comment --- packages/xo-web/src/xo-app/jobs/overview/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/xo-web/src/xo-app/jobs/overview/index.js b/packages/xo-web/src/xo-app/jobs/overview/index.js index 66156f4db37..51afd976d20 100644 --- a/packages/xo-web/src/xo-app/jobs/overview/index.js +++ b/packages/xo-web/src/xo-app/jobs/overview/index.js @@ -128,7 +128,7 @@ export default class Overview extends Component { }) const unsubscribeSchedules = subscribeSchedules(schedules => { - // Get only backup jobs. + // Get only generic jobs schedules = filter(schedules, schedule => { const job = this.state.jobs[schedule.jobId] return job && jobKeyToLabel[job.key] From 3973ff43165768da6b2ebd2e7bad1d5819677c87 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Tue, 12 Sep 2023 10:46:25 +0200 Subject: [PATCH 3/8] chore(xo-web/_getScheduleJob): explicit comparison --- packages/xo-web/src/xo-app/jobs/overview/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/xo-web/src/xo-app/jobs/overview/index.js b/packages/xo-web/src/xo-app/jobs/overview/index.js index 51afd976d20..fa405896e14 100644 --- a/packages/xo-web/src/xo-app/jobs/overview/index.js +++ b/packages/xo-web/src/xo-app/jobs/overview/index.js @@ -131,7 +131,7 @@ export default class Overview extends Component { // Get only generic jobs schedules = filter(schedules, schedule => { const job = this.state.jobs[schedule.jobId] - return job && jobKeyToLabel[job.key] + return job !== undefined && job.key in jobKeyToLabel }) this.setState({ From b69bf313d7d66fb766445d9a49f1d73d963877f1 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Tue, 12 Sep 2023 14:24:45 +0200 Subject: [PATCH 4/8] chore(xo-web/_getScheduleJob): remove unnecessary sort --- packages/xo-web/src/xo-app/jobs/overview/index.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/xo-web/src/xo-app/jobs/overview/index.js b/packages/xo-web/src/xo-app/jobs/overview/index.js index fa405896e14..d65ddc49770 100644 --- a/packages/xo-web/src/xo-app/jobs/overview/index.js +++ b/packages/xo-web/src/xo-app/jobs/overview/index.js @@ -12,7 +12,7 @@ import { addSubscriptions } from 'utils' import { Container } from 'grid' import { createSelector } from 'selectors' import { Card, CardHeader, CardBlock } from 'card' -import { filter, find, forEach, orderBy } from 'lodash' +import { filter, find, forEach } from 'lodash' import { deleteSchedule, deleteSchedules, @@ -134,9 +134,7 @@ export default class Overview extends Component { return job !== undefined && job.key in jobKeyToLabel }) - this.setState({ - schedules: orderBy(schedules, schedule => +schedule.id.split(':')[1], ['desc']), - }) + this.setState({ schedules }) }) this.componentWillUnmount = () => { From b9d45a9b8d101943c21f53840b6e2d33566b9352 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Tue, 12 Sep 2023 14:25:15 +0200 Subject: [PATCH 5/8] fix(xo-web/jobs): make schedules a computed Fixes #6968 The schedules did not appear if the jobs subscription triggered after the schedules one. The logic has been moved to a computed depending on both subscriptions. --- CHANGELOG.unreleased.md | 1 + .../xo-web/src/xo-app/jobs/overview/index.js | 24 +++++++++++-------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index 42ddf0489ee..3d2fd4f41e6 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -15,6 +15,7 @@ - [Backup/Restore] Fix `Cannot read properties of undefined (reading 'id')` error when restoring via an XO Proxy (PR [#7026](https://github.com/vatesfr/xen-orchestra/pull/7026)) - [Google/GitHub Auth] Fix `Internal Server Error` (xo-server: `Cannot read properties of undefined (reading 'id')`) when logging in with Google or GitHub [Forum#7729](https://xcp-ng.org/forum/topic/7729) (PRs [#7031](https://github.com/vatesfr/xen-orchestra/pull/7031) [#7032](https://github.com/vatesfr/xen-orchestra/pull/7032)) +- [Jobs] Fix schedules not being displayed on first load [#6968](https://github.com/vatesfr/xen-orchestra/issues/6968) (PR [#7034](https://github.com/vatesfr/xen-orchestra/pull/7034)) ### Packages to release diff --git a/packages/xo-web/src/xo-app/jobs/overview/index.js b/packages/xo-web/src/xo-app/jobs/overview/index.js index d65ddc49770..9c2d798e319 100644 --- a/packages/xo-web/src/xo-app/jobs/overview/index.js +++ b/packages/xo-web/src/xo-app/jobs/overview/index.js @@ -128,12 +128,6 @@ export default class Overview extends Component { }) const unsubscribeSchedules = subscribeSchedules(schedules => { - // Get only generic jobs - schedules = filter(schedules, schedule => { - const job = this.state.jobs[schedule.jobId] - return job !== undefined && job.key in jobKeyToLabel - }) - this.setState({ schedules }) }) @@ -143,8 +137,20 @@ export default class Overview extends Component { } } - _getIsScheduleUserMissing = createSelector( + _getGenericSchedules = createSelector( () => this.state.schedules, + () => this.state.jobs, + + // Get only generic jobs + (schedules, jobs) => + filter(schedules, schedule => { + const job = jobs[schedule.jobId] + return job !== undefined && job.key in jobKeyToLabel + }) + ) + + _getIsScheduleUserMissing = createSelector( + this._getGenericSchedules, () => this.props.users, () => this.state.jobs, (schedules, users, jobs) => { @@ -178,8 +184,6 @@ export default class Overview extends Component { ] render() { - const { schedules } = this.state - return process.env.XOA_PLAN > 3 ? ( @@ -189,7 +193,7 @@ export default class Overview extends Component { Date: Tue, 12 Sep 2023 12:08:56 +0200 Subject: [PATCH 6/8] feat(xo-web/addSubscriptions): support initial values --- .../xo-web/src/common/add-subscriptions.js | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/xo-web/src/common/add-subscriptions.js b/packages/xo-web/src/common/add-subscriptions.js index 43d868668d8..5f092e3cd77 100644 --- a/packages/xo-web/src/common/add-subscriptions.js +++ b/packages/xo-web/src/common/add-subscriptions.js @@ -1,20 +1,31 @@ import React from 'react' -import { map, mapValues, noop } from 'lodash' +import { forOwn, map } from 'lodash' const call = fn => fn() // `subscriptions` can be a function if we want to ensure that the subscription // callbacks have been correctly initialized when there are circular dependencies +// +// each subscription can be either a `subscribe` function or a `[subscribe, initialValue]` array const addSubscriptions = subscriptions => Component => class SubscriptionWrapper extends React.PureComponent { constructor(props) { super(props) // provide all props since the beginning (better behavior with Freactal) - this.state = mapValues( - (this._subscribes = typeof subscriptions === 'function' ? subscriptions(props) : subscriptions), - noop - ) + const state = {} + const subscribes = {} + forOwn(typeof subscriptions === 'function' ? subscriptions(props) : subscriptions, (subscription, prop) => { + if (typeof subscription === 'function') { + subscribes[prop] = subscription + state[prop] = undefined + } else { + subscribes[prop] = subscription[0] + state[prop] = subscription[1] + } + }) + this.state = state + this._subscribes = subscribes } _unsubscribes = undefined From f89e5789731f3452e5cb9f78383a8dfaaacf5c24 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Tue, 12 Sep 2023 12:10:33 +0200 Subject: [PATCH 7/8] chore(xo-web/jobs): use addSubscriptions for all subs --- .../xo-web/src/xo-app/jobs/overview/index.js | 44 ++++--------------- 1 file changed, 8 insertions(+), 36 deletions(-) diff --git a/packages/xo-web/src/xo-app/jobs/overview/index.js b/packages/xo-web/src/xo-app/jobs/overview/index.js index 9c2d798e319..0a3eead483e 100644 --- a/packages/xo-web/src/xo-app/jobs/overview/index.js +++ b/packages/xo-web/src/xo-app/jobs/overview/index.js @@ -12,7 +12,7 @@ import { addSubscriptions } from 'utils' import { Container } from 'grid' import { createSelector } from 'selectors' import { Card, CardHeader, CardBlock } from 'card' -import { filter, find, forEach } from 'lodash' +import { filter, find, forEach, keyBy } from 'lodash' import { deleteSchedule, deleteSchedules, @@ -100,46 +100,18 @@ const ACTIONS = [ // =================================================================== @addSubscriptions({ - users: subscribeUsers, + jobs: [cb => subscribeJobs(jobs => cb(keyBy(jobs, 'id'))), {}], + schedules: [subscribeSchedules, []], + users: [subscribeUsers, []], }) export default class Overview extends Component { static contextTypes = { router: PropTypes.object, } - constructor(props) { - super(props) - this.state = { - jobs: {}, - schedules: [], - } - } - - componentWillMount() { - const unsubscribeJobs = subscribeJobs(jobs => { - const obj = {} - forEach(jobs, job => { - obj[job.id] = job - }) - - this.setState({ - jobs: obj, - }) - }) - - const unsubscribeSchedules = subscribeSchedules(schedules => { - this.setState({ schedules }) - }) - - this.componentWillUnmount = () => { - unsubscribeJobs() - unsubscribeSchedules() - } - } - _getGenericSchedules = createSelector( - () => this.state.schedules, - () => this.state.jobs, + () => this.props.schedules, + () => this.props.jobs, // Get only generic jobs (schedules, jobs) => @@ -152,7 +124,7 @@ export default class Overview extends Component { _getIsScheduleUserMissing = createSelector( this._getGenericSchedules, () => this.props.users, - () => this.state.jobs, + () => this.props.jobs, (schedules, users, jobs) => { const isScheduleUserMissing = {} @@ -196,7 +168,7 @@ export default class Overview extends Component { collection={this._getGenericSchedules()} columns={SCHEDULES_COLUMNS} data-isScheduleUserMissing={this._getIsScheduleUserMissing()} - data-jobs={this.state.jobs} + data-jobs={this.props.jobs} individualActions={this._individualActions} shortcutsTarget='body' stateUrlParam='s' From f3610b16f87d38d452e1de9f789c5c4056d54706 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Tue, 12 Sep 2023 12:28:58 +0200 Subject: [PATCH 8/8] chore(xo-web/jobs): use set for user ids --- packages/xo-web/src/xo-app/jobs/overview/index.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/xo-web/src/xo-app/jobs/overview/index.js b/packages/xo-web/src/xo-app/jobs/overview/index.js index 0a3eead483e..fc80e2c70e3 100644 --- a/packages/xo-web/src/xo-app/jobs/overview/index.js +++ b/packages/xo-web/src/xo-app/jobs/overview/index.js @@ -12,7 +12,7 @@ import { addSubscriptions } from 'utils' import { Container } from 'grid' import { createSelector } from 'selectors' import { Card, CardHeader, CardBlock } from 'card' -import { filter, find, forEach, keyBy } from 'lodash' +import { filter, forEach, keyBy } from 'lodash' import { deleteSchedule, deleteSchedules, @@ -102,7 +102,7 @@ const ACTIONS = [ @addSubscriptions({ jobs: [cb => subscribeJobs(jobs => cb(keyBy(jobs, 'id'))), {}], schedules: [subscribeSchedules, []], - users: [subscribeUsers, []], + userIds: [cb => subscribeUsers(users => cb(new Set(users.map(_ => _.id)))), new Set()], }) export default class Overview extends Component { static contextTypes = { @@ -123,13 +123,13 @@ export default class Overview extends Component { _getIsScheduleUserMissing = createSelector( this._getGenericSchedules, - () => this.props.users, + () => this.props.userIds, () => this.props.jobs, - (schedules, users, jobs) => { + (schedules, userIds, jobs) => { const isScheduleUserMissing = {} forEach(schedules, schedule => { - isScheduleUserMissing[schedule.id] = !find(users, user => user.id === jobs[schedule.jobId].userId) + isScheduleUserMissing[schedule.id] = !userIds.has(jobs[schedule.jobId].userId) }) return isScheduleUserMissing