From 334f850d4c0ad1eae41d2c988808cda2684b722d Mon Sep 17 00:00:00 2001 From: lorenzo-cavazzi Date: Mon, 24 Jun 2019 15:21:57 +0200 Subject: [PATCH] fix branch and commit selection logic --- src/api-client/notebook-servers.js | 3 +- src/notebooks/Notebooks.container.js | 83 +++++++++++++++++----------- src/notebooks/Notebooks.present.js | 3 +- src/notebooks/Notebooks.state.js | 65 ++++++++++++---------- 4 files changed, 89 insertions(+), 65 deletions(-) diff --git a/src/api-client/notebook-servers.js b/src/api-client/notebook-servers.js index 718b711de6..5377159c4f 100644 --- a/src/api-client/notebook-servers.js +++ b/src/api-client/notebook-servers.js @@ -91,13 +91,14 @@ function addNotebookServersMethods(client) { }); } - client.startNotebook = (projectUrl, commitId, options) => { + client.startNotebook = (projectUrl, branchName, commitId, options) => { const headers = client.getBasicHeaders(); const url = `${client.baseUrl}/notebooks/${projectUrl}/${commitId}`; return client.clientFetch(url, { method: 'POST', headers, + queryParams: { branch: branchName }, body: JSON.stringify(options) }).then((resp) => { return resp.data; diff --git a/src/notebooks/Notebooks.container.js b/src/notebooks/Notebooks.container.js index 028b8c5aa6..62cd94c031 100644 --- a/src/notebooks/Notebooks.container.js +++ b/src/notebooks/Notebooks.container.js @@ -61,6 +61,7 @@ class StartNotebookServer extends Component { componentDidMount() { this._isMounted = true; + this.startNotebookPolling(); this.refreshBranches(); } @@ -70,34 +71,22 @@ class StartNotebookServer extends Component { } componentDidUpdate(previousProps) { - // TODO: this is a temporary fix, remove it once the component won't be rerendered multiple times at first load + // TODO: this is a temporary fix, remove it once the component won't be + // rerendered multiple times at the first url load if (this.state.first && StatusHelper.isUpdating(previousProps.branches) && !StatusHelper.isUpdating(this.props.branches)) { - if (this.props.branches.length === 1) { - this.setBranch(this.props.branches[0]); - } - else { - this.validateBranch(this.props.branches); - } + this.autoselectBranch(this.props.branches); this.setState({ first: false }); } } refreshBranches() { const { branches } = this.props; - if (!StatusHelper.isUpdating(branches)) { - this.props.refreshBranches().then((branches) => { - if (this._isMounted) { - if (branches.length === 1) { - this.setBranch(branches[0]); - } - else { - this.validateBranch(branches); - } - } - }); - } + if (StatusHelper.isUpdating(branches)) return; + this.props.refreshBranches().then((branches) => { + this.autoselectBranch(branches); + }); } setBranch(branch) { @@ -145,29 +134,39 @@ class StartNotebookServer extends Component { return false; } + autoselectBranch(branches, defaultBranch = "master") { + if (this._isMounted) { + const branch = this.model.get("filters.branch"); + if (!branch.name) { + const autoSelect = branches.filter(branch => branch.name === defaultBranch); + if (autoSelect.length !== 1) return; // improve this logic if necessary when defaultBranch will be dynamic + this.setBranch(autoSelect[0]); + } + else { + this.validateBranch(branches); + } + } + } + refreshCommits(updatedBranch) { const commits = this.model.get("data.commits"); - if (!StatusHelper.isUpdating(commits)) { - const { projectId } = this.props; - const branch = updatedBranch && typeof updatedBranch === "string" ? - updatedBranch : - this.model.get("filters.branch"); - this.model.fetchCommits(projectId, branch.name).then((commits) => { - if (this._isMounted) { - this.validateCommit(commits); - } - }); - } + if (StatusHelper.isUpdating(commits)) return; + const { projectId } = this.props; + const branch = updatedBranch && typeof updatedBranch === "string" ? + updatedBranch : + this.model.get("filters.branch"); + this.model.fetchCommits(projectId, branch.name).then((commits) => { + this.autoselectCommit(commits); + }); } setCommit(commit) { const oldCommit = this.model.get("filters.commit"); - if (!commit.id || commit.id === oldCommit.id) { - this.model.stopNotebookPolling(); + if (commit.id === oldCommit.id) { return; } this.model.setCommit(commit); - this.startNotebookPolling(); + this.model.verifyIfRunning(this.props.projectId, this.props.projectPath); } setCommitFromId(eventOrId) { @@ -200,6 +199,8 @@ class StartNotebookServer extends Component { commits; for (let commitCurrent of filteredCommits) { if (commit.id === commitCurrent.id) { + // necessary for istant refresh in the UI + this.model.verifyIfRunning(this.props.projectId, this.props.projectPath); return true; } } @@ -208,6 +209,22 @@ class StartNotebookServer extends Component { return false; } + autoselectCommit(commits, defaultCommit = 0) { + if (this._isMounted) { + if (this._isMounted) { + const commit = this.model.get("filters.commit"); + if (!commit.id) { + const autoSelect = commits[defaultCommit]; + if (!autoSelect) return; // improve this logic if "latest" won't be the default anymore + this.setCommit(autoSelect); + } + else { + this.validateCommit(commits); + } + } + } + } + setServerOptionFromEvent(option, event) { const value = event.target.checked !== undefined ? event.target.checked: diff --git a/src/notebooks/Notebooks.present.js b/src/notebooks/Notebooks.present.js index af0cb40b48..1708de194d 100644 --- a/src/notebooks/Notebooks.present.js +++ b/src/notebooks/Notebooks.present.js @@ -546,7 +546,8 @@ class StartNotebookCommitsOptions extends Component { class StartNotebookOptions extends Component { render() { const { commit } = this.props.filters; - if (!commit.id) { + const { branches, commits } = this.props.data; + if (!commit.id || StatusHelper.isUpdating(branches) || StatusHelper.isUpdating(commits)) { return null; } const { justStarted } = this.props; diff --git a/src/notebooks/Notebooks.state.js b/src/notebooks/Notebooks.state.js index d114640efa..2551737f55 100644 --- a/src/notebooks/Notebooks.state.js +++ b/src/notebooks/Notebooks.state.js @@ -112,16 +112,15 @@ class NotebooksModel extends StateModel { for (let notebookName of Object.keys(notebooks)) { const notebook = notebooks[notebookName]; const annotations = cleanAnnotations(notebook["annotations"], "renku.io"); - if (parseInt(annotations.projectId) === projectId) { - if (annotations["branch"] === branch.name && annotations["commit-sha"] === commit.id) { - this.setObject({ notebooks: { - status: notebook.ready ? - "running" : - notebook.pending, - url: notebook.url - }}); - return true; - } + if (parseInt(annotations.projectId) !== projectId) continue; + if (annotations["branch"] === branch.name && annotations["commit-sha"] === commit.id) { + this.setObject({ notebooks: { + status: notebook.ready ? + "running" : + notebook.pending, + url: notebook.url + }}); + return true; } } this.setObject({notebooks: { @@ -134,6 +133,18 @@ class NotebooksModel extends StateModel { return false; } + notebookPollingIteration(verifyRunningId, projectPath) { + const fetchPromise = this.fetchNotebooks(); + if (verifyRunningId) { + return fetchPromise.then((servers) => { + return this.verifyIfRunning(verifyRunningId, projectPath, servers); + }); + } + else { + return fetchPromise; + } + } + startNotebookPolling(verifyRunningId, projectPath) { if (verifyRunningId) { this.setObject({notebooks: { @@ -144,28 +155,12 @@ class NotebooksModel extends StateModel { const oldPoller = this.get('notebooks.polling'); if (oldPoller == null) { const newPoller = setInterval(() => { - const fetchPromise = this.fetchNotebooks(); - if (verifyRunningId) { - return fetchPromise.then((servers) => { - return this.verifyIfRunning(verifyRunningId, projectPath, servers); - }); - } - else { - return fetchPromise; - } + this.notebookPollingIteration(verifyRunningId, projectPath); }, 3000); this.set('notebooks.polling', newPoller); // fetch immediatly - const fetchPromise = this.fetchNotebooks(true); - if (verifyRunningId) { - return fetchPromise.then((servers) => { - return this.verifyIfRunning(verifyRunningId, projectPath, servers); - }); - } - else { - return fetchPromise; - } + this.notebookPollingIteration(verifyRunningId, projectPath); } } @@ -195,10 +190,20 @@ class NotebooksModel extends StateModel { this.set(`data.selectedOptions.${option}`, value); } - startServer(projectPath, commitId) { + startServer(projectPath, branchName, commitId) { const options = { serverOptions: this.get('data.selectedOptions') }; + let branch = branchName; + if (!branchName) { + const selctedBranch = this.get('filters.branch'); + if (selctedBranch.name) { + branch = selctedBranch.name; + } + else { + branch = "master"; + } + } let commit = commitId; if (!commitId) { const selctedCommit = this.get('filters.commit'); @@ -211,7 +216,7 @@ class NotebooksModel extends StateModel { } this.set('notebooks.status', 'spawn'); - return this.client.startNotebook(projectPath, commit, options); + return this.client.startNotebook(projectPath, branch, commit, options); } stopNotebookPolling() {