From 70dbdeded3281912c20bbf782a4ea9eba19864f4 Mon Sep 17 00:00:00 2001 From: Maksym Mykhailenko Date: Thu, 27 Feb 2020 20:38:35 +0800 Subject: [PATCH 1/5] feat: show refresh dialog when looker session is expired --- src/config/constants.js | 7 +- src/projects/actions/projectReports.js | 13 ++-- .../ProjectSummaryReportContainer.jsx | 77 +++++++++++++------ src/projects/reducers/projectReports.js | 24 ++++-- 4 files changed, 85 insertions(+), 36 deletions(-) diff --git a/src/config/constants.js b/src/config/constants.js index d5e695f10..b22893a59 100644 --- a/src/config/constants.js +++ b/src/config/constants.js @@ -417,7 +417,7 @@ export const LOAD_PROJECT_SUMMARY = 'LOAD_PROJECT_SUMMARY' export const LOAD_PROJECT_SUMMARY_PENDING = 'LOAD_PROJECT_SUMMARY_PENDING' export const LOAD_PROJECT_SUMMARY_SUCCESS = 'LOAD_PROJECT_SUMMARY_SUCCESS' export const LOAD_PROJECT_SUMMARY_FAILURE = 'LOAD_PROJECT_SUMMARY_FAILURE' -export const REFRESH_LOOKER_SESSION = 'REFRESH_LOOKER_SESSION' +export const SET_LOOKER_SESSION_EXPIRED = 'SET_LOOKER_SESSION_EXPIRED' // Product attachments export const ADD_PRODUCT_ATTACHMENT = 'ADD_PRODUCT_ATTACHMENT' @@ -962,4 +962,7 @@ export const PROJECT_REPORTS = { TAAS_MEMBERS : 'taas_members', } -export const REPORT_SESSION_LENGTH = 1800 \ No newline at end of file +/** + * Report session length in seconds + */ +export const REPORT_SESSION_LENGTH = 10 * 60 // 10 minutes diff --git a/src/projects/actions/projectReports.js b/src/projects/actions/projectReports.js index 7c91a88b7..42cbdffee 100644 --- a/src/projects/actions/projectReports.js +++ b/src/projects/actions/projectReports.js @@ -1,6 +1,6 @@ import { LOAD_PROJECT_SUMMARY, - REFRESH_LOOKER_SESSION, + SET_LOOKER_SESSION_EXPIRED, } from '../../config/constants' import { getProjectSummary, @@ -33,14 +33,15 @@ export function loadProjectReportsUrls(projectId, reportName) { } /** - * Redux action to refresh the looker session. It is aimed to just indicate that there is need - * of refreshing the token. It does not do any thing itself. It is upto the state listner to react. + * Redux action set the flag `lookerSessionExpired` + * + * @param {Boolean} isExpired true to indicate that looker session is expired */ -export function refreshLookerSession() { +export function setLookerSessionExpired(isExpired) { return (dispatch) => { return dispatch({ - type: REFRESH_LOOKER_SESSION, - payload: { lookerSessionExpired: true } + type: SET_LOOKER_SESSION_EXPIRED, + payload: { lookerSessionExpired: isExpired } }) } } \ No newline at end of file diff --git a/src/projects/detail/containers/ProjectSummaryReportContainer.jsx b/src/projects/detail/containers/ProjectSummaryReportContainer.jsx index 9533a0430..2eece0c4a 100644 --- a/src/projects/detail/containers/ProjectSummaryReportContainer.jsx +++ b/src/projects/detail/containers/ProjectSummaryReportContainer.jsx @@ -4,6 +4,7 @@ import PT from 'prop-types' import { connect } from 'react-redux' import { withRouter } from 'react-router-dom' import MediaQuery from 'react-responsive' +import Modal from 'react-modal' import { SCREEN_BREAKPOINT_MD, @@ -17,7 +18,7 @@ import Sticky from '../../../components/Sticky' import ProjectInfoContainer from './ProjectInfoContainer' import PERMISSIONS from '../../../config/permissions' import { checkPermission } from '../../../helpers/permissions' -import { loadProjectSummary, loadProjectReportsUrls, refreshLookerSession } from '../../actions/projectReports' +import { loadProjectReportsUrls, setLookerSessionExpired } from '../../actions/projectReports' import spinnerWhileLoading from '../../../components/LoadingSpinner' import './ProjectSummaryReportContainer.scss' @@ -34,34 +35,50 @@ class ProjectSummaryReportContainer extends React.Component { constructor(props) { super(props) + this.timer = null this.setLookerSessionTimer = this.setLookerSessionTimer.bind(this) + this.reloadProjectReport = this.reloadProjectReport.bind(this) + } + + reloadProjectReport() { + this.props.loadProjectReportsUrls(_.get(this.props, 'project.id'), PROJECT_REPORTS.PROJECT_SUMMARY) + // don't have to set session expire timer here, it would be set of iframe load + } + + componentWillMount() { + this.reloadProjectReport() + // don't have to set session expire timer here, it would be set of iframe load + } + + componentWillUnmount() { + if (this.timer) { + clearTimeout(this.timer) + } } componentWillUpdate(nextProps) { const nextReportProjectId = _.get(nextProps, 'reportsProjectId') const nextProjectId = _.get(nextProps, 'project.id') - const lookerSessionExpired = !this.props.lookerSessionExpired && nextProps.lookerSessionExpired - if(lookerSessionExpired || (nextProjectId && nextReportProjectId !== nextProjectId)) { - nextProps.loadProjectReportsUrls(nextProjectId, PROJECT_REPORTS.PROJECT_SUMMARY) - this.setLookerSessionTimer() + + if (nextProjectId && nextReportProjectId !== nextProjectId) { + this.props.loadProjectReportsUrls(nextProjectId, PROJECT_REPORTS.PROJECT_SUMMARY) + // don't have to set session expire timer here, it would be set of iframe load } } setLookerSessionTimer() { console.log('Setting Looker Session Timer') + if (this.timer) { clearTimeout(this.timer) } - let timeoutDuration = 60*1000 - if (REPORT_SESSION_LENGTH > 2*60) { - timeoutDuration = REPORT_SESSION_LENGTH*1000 - 2*60*1000 - } - // set timeout for raising alert to refresh the token 2 minutes before the session expire + + // set timeout for raising alert to refresh the token when session expires this.timer = setTimeout(() => { - console.log('Calling refresh looker session action') - this.props.refreshLookerSession() - }, (timeoutDuration)) + console.log('Looker Session is expired by timer') + this.props.setLookerSessionExpired(true) + }, REPORT_SESSION_LENGTH * 1000) } render() { @@ -78,6 +95,7 @@ class ProjectSummaryReportContainer extends React.Component { isLoading, location, projectSummaryEmbedUrl, + lookerSessionExpired, } = this.props const leftArea = ( @@ -111,13 +129,29 @@ class ProjectSummaryReportContainer extends React.Component { - { - - } + +
+ Report sessions expired +
+ +
+ To keep the data up to date, please, hit "Refresh" button to reload the report. +
+ +
+ +
+ + ) @@ -154,9 +188,8 @@ const mapStateToProps = ({ projectState, projectTopics, phasesTopics, projectRep } const mapDispatchToProps = { - loadProjectSummary, loadProjectReportsUrls, - refreshLookerSession, + setLookerSessionExpired, } export default connect(mapStateToProps, mapDispatchToProps)(withRouter(ProjectSummaryReportContainer)) diff --git a/src/projects/reducers/projectReports.js b/src/projects/reducers/projectReports.js index c669904b8..73046e7b5 100644 --- a/src/projects/reducers/projectReports.js +++ b/src/projects/reducers/projectReports.js @@ -2,7 +2,7 @@ import { LOAD_PROJECT_SUMMARY_PENDING, LOAD_PROJECT_SUMMARY_SUCCESS, LOAD_PROJECT_SUMMARY_FAILURE, - REFRESH_LOOKER_SESSION, + SET_LOOKER_SESSION_EXPIRED, } from '../../config/constants' const initialState = { @@ -14,6 +14,19 @@ const initialState = { lookerSessionExpired: false, } +/** + * Adds a `random` query param to the URL so browser could treat such a URL as different. + * + * @param {String} url URL string to augment + * + * @returns {String} URL with `random` query param + */ +function addRandomParamToUrl(url) { + const randomParam = `random=${Math.random().toString().slice(2)}` + + return url + (url.indexOf('?') > -1 ? '&' : '?') + randomParam +} + export const projectReports = function (state=initialState, action) { const payload = action.payload @@ -22,7 +35,7 @@ export const projectReports = function (state=initialState, action) { return Object.assign({}, state, { isLoading: true, error: false, - projectId: action.meta.projectId + projectId: action.meta.projectId, }) case LOAD_PROJECT_SUMMARY_SUCCESS: @@ -30,7 +43,7 @@ export const projectReports = function (state=initialState, action) { return Object.assign({}, state, { isLoading: false, error: false, - projectSummaryEmbedUrl: payload, + projectSummaryEmbedUrl: addRandomParamToUrl(payload), lookerSessionExpired: false, // projectSummary: payload }) @@ -41,14 +54,13 @@ export const projectReports = function (state=initialState, action) { case LOAD_PROJECT_SUMMARY_FAILURE: { return Object.assign({}, state, { isLoading: false, - lookerSessionExpired: false, error: payload }) } - case REFRESH_LOOKER_SESSION: { + case SET_LOOKER_SESSION_EXPIRED: { return Object.assign({}, state, { - lookerSessionExpired: true + lookerSessionExpired: payload }) } From 7b22f30dbc26ceb557c6982d4224d840c8d9eb4a Mon Sep 17 00:00:00 2001 From: Maksym Mykhailenko Date: Thu, 27 Feb 2020 20:43:44 +0800 Subject: [PATCH 2/5] fix: set looker session expiration time to 30 minutes --- src/config/constants.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config/constants.js b/src/config/constants.js index b22893a59..2c5611701 100644 --- a/src/config/constants.js +++ b/src/config/constants.js @@ -965,4 +965,4 @@ export const PROJECT_REPORTS = { /** * Report session length in seconds */ -export const REPORT_SESSION_LENGTH = 10 * 60 // 10 minutes +export const REPORT_SESSION_LENGTH = 30 * 60 // 30 minutes From 88ad7db0157f742a147129c9c5de85c6188134e9 Mon Sep 17 00:00:00 2001 From: Maksym Mykhailenko Date: Mon, 2 Mar 2020 12:22:36 +0800 Subject: [PATCH 3/5] fix: explicitly retrieve invites on project listing page --- src/api/projects.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/projects.js b/src/api/projects.js index 559b5d55d..63563c4a1 100644 --- a/src/api/projects.js +++ b/src/api/projects.js @@ -4,7 +4,7 @@ import { PROJECTS_API_URL, PROJECTS_LIST_PER_PAGE } from '../config/constants' export function getProjects(criteria, pageNum) { // add default params - const includeFields = ['id', 'name', 'description', 'members', 'status', 'type', 'actualPrice', 'estimatedPrice', 'createdAt', 'updatedAt', 'createdBy', 'updatedBy', 'details', 'lastActivityAt', 'lastActivityUserId', 'version', 'templateId'] + const includeFields = ['id', 'name', 'description', 'members', 'invites', 'status', 'type', 'actualPrice', 'estimatedPrice', 'createdAt', 'updatedAt', 'createdBy', 'updatedBy', 'details', 'lastActivityAt', 'lastActivityUserId', 'version', 'templateId'] const params = { fields: includeFields.join(','), sort: 'updatedAt+desc', // default sort value From 29f0a736272f3a01d2118d2a74bb676f8e6c2fee Mon Sep 17 00:00:00 2001 From: Maksym Mykhailenko Date: Mon, 2 Mar 2020 13:05:18 +0800 Subject: [PATCH 4/5] fix: explicitly request emails for members --- src/api/projectMembers.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/api/projectMembers.js b/src/api/projectMembers.js index 8ceba034b..007f188b0 100644 --- a/src/api/projectMembers.js +++ b/src/api/projectMembers.js @@ -68,7 +68,7 @@ export function removeProjectMember(projectId, memberId) { } export function getProjectMembers(projectId) { - const fields = 'id,userId,role,isPrimary,deletedAt,createdAt,updatedAt,deletedBy,createdBy,updatedBy,handle,firstName,lastName,photoURL,workingHourStart,workingHourEnd,timeZone' + const fields = 'id,userId,role,isPrimary,deletedAt,createdAt,updatedAt,deletedBy,createdBy,updatedBy,handle,firstName,lastName,email,photoURL,workingHourStart,workingHourEnd,timeZone' const url = `${PROJECTS_API_URL}/v5/projects/${projectId}/members/?fields=` + encodeURIComponent(fields) return axios.get(url) @@ -78,7 +78,7 @@ export function getProjectMembers(projectId) { } export function getProjectMember(projectId, memberId) { - const fields = 'id,userId,role,isPrimary,deletedAt,createdAt,updatedAt,deletedBy,createdBy,updatedBy,handle,firstName,lastName,photoURL,workingHourStart,workingHourEnd,timeZone' + const fields = 'id,userId,role,isPrimary,deletedAt,createdAt,updatedAt,deletedBy,createdBy,updatedBy,handle,firstName,lastName,email,photoURL,workingHourStart,workingHourEnd,timeZone' const url = `${PROJECTS_API_URL}/v5/projects/${projectId}/members/${memberId}?fields=` + encodeURIComponent(fields) return axios.get(url) From c8be93167eb62345a54c196b0b5d392fa640de34 Mon Sep 17 00:00:00 2001 From: Maksym Mykhailenko Date: Tue, 3 Mar 2020 11:45:40 +0800 Subject: [PATCH 5/5] fix: don't use random for report URL ref issue #3727 --- .../containers/ProjectSummaryReportContainer.jsx | 11 +++++++++++ src/projects/reducers/projectReports.js | 15 +-------------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/projects/detail/containers/ProjectSummaryReportContainer.jsx b/src/projects/detail/containers/ProjectSummaryReportContainer.jsx index 2eece0c4a..b28ae1e22 100644 --- a/src/projects/detail/containers/ProjectSummaryReportContainer.jsx +++ b/src/projects/detail/containers/ProjectSummaryReportContainer.jsx @@ -36,6 +36,9 @@ class ProjectSummaryReportContainer extends React.Component { constructor(props) { super(props) + this.state = { + iframeKey: 0, // we would use it to force iframe to reload + } this.timer = null this.setLookerSessionTimer = this.setLookerSessionTimer.bind(this) this.reloadProjectReport = this.reloadProjectReport.bind(this) @@ -65,6 +68,13 @@ class ProjectSummaryReportContainer extends React.Component { this.props.loadProjectReportsUrls(nextProjectId, PROJECT_REPORTS.PROJECT_SUMMARY) // don't have to set session expire timer here, it would be set of iframe load } + + // when we get a new URL for report, force iframe to reload, in case the URL stays the same + if (this.props.isLoading && !nextProps.isLoading) { + this.setState({ + iframeKey: this.state.iframeKey + 1 + }) + } } setLookerSessionTimer() { @@ -148,6 +158,7 @@ class ProjectSummaryReportContainer extends React.Component { -1 ? '&' : '?') + randomParam -} - export const projectReports = function (state=initialState, action) { const payload = action.payload @@ -43,7 +30,7 @@ export const projectReports = function (state=initialState, action) { return Object.assign({}, state, { isLoading: false, error: false, - projectSummaryEmbedUrl: addRandomParamToUrl(payload), + projectSummaryEmbedUrl: payload, lookerSessionExpired: false, // projectSummary: payload })