Skip to content
This repository has been archived by the owner on Dec 13, 2020. It is now read-only.

Dev 979 #1016

Merged
merged 3 commits into from
Jul 13, 2017
Merged

Dev 979 #1016

Show file tree
Hide file tree
Changes from 2 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
70 changes: 45 additions & 25 deletions src/actions/MenuActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ import axios from 'axios';

// REQUESTS

let breadcrumbsCache = {};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question (mind that i'm a javascript-noob): how and when is this cache discarded or invalidated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need to invalidate it. when you refresh the page, it gets cleared. when you 're navigating, it's not changed.

Copy link
Member

@teosarca teosarca Jul 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wiadev here is my input as yet another javascript-noob :)

Loud thinking: i think the frontend side shall not cache those results at all. It shall simply call the endpoint each time user is pressing on the breadcrumb item.
Reasons:

  • we are already caching it on backend side and it would be quite difficult and error prone to invalidate all caches along the road, when a sysadm is changing the menu.
  • when and if we would want to optimize that part and save some calls, we can do browser level caching (ETag, cache control etc), so basically, when the backend menu cache is invalidated we would send a different ETag.

wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@teosarca Removed the cache.

All things you told above are correct and valid. But this particular piece simply didn't get updated that often. That's why I decided to reduce requests.

Now it removed and just don't fire duplicate requests sent for the same breadcrumb items.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

koszi :)

let breadcrumbsRequested = false;

export function pathRequest(nodeId) {
return axios.get(
config.API_URL +
Expand Down Expand Up @@ -78,33 +81,50 @@ export function getRootBreadcrumb() {

export function getWindowBreadcrumb(id){
return dispatch => {
elementPathRequest('window', id).then(response => {
let req = 0;
let pathData = flattenOneLine(response.data);

// promise to get all of the breadcrumb menu options
let breadcrumbProcess = new Promise((resolve) => {

for(let i = 0; i < pathData.length; i++){
const node = pathData[i];

breadcrumbRequest(node.nodeId).then(item => {
node.children = item.data;
req += 1;

if(req === pathData.length){
resolve(pathData);
if (!breadcrumbsRequested) {
breadcrumbsRequested = true;
elementPathRequest('window', id).then(response => {
let req = 0;
let pathData = flattenOneLine(response.data);

// promise to get all of the breadcrumb menu options
let breadcrumbProcess = new Promise((resolve) => {

for(let i = 0; i < pathData.length; i++){
const node = pathData[i];
let nodeId = node.nodeId;

if (typeof breadcrumbsCache[nodeId] !== 'undefined') {
node.children = breadcrumbsCache[nodeId];
req += 1;

if(req === pathData.length){
resolve(pathData);
}
}
else {
breadcrumbRequest(nodeId).then(item => {
node.children = item.data;
req += 1;
breadcrumbsCache[nodeId] = item.data;

if(req === pathData.length){
resolve(pathData);
}
})
}
})
}
}
});

return breadcrumbProcess;
}).then((item) => {
dispatch(setBreadcrumb(item.reverse()));
breadcrumbsRequested = false;
}).catch(() => {
dispatch(setBreadcrumb([]));
breadcrumbsRequested = false;
});

return breadcrumbProcess;
}).then((item) => {
dispatch(setBreadcrumb(item.reverse()));
}).catch(() => {
dispatch(setBreadcrumb([]));
});
}
}
}

Expand Down
13 changes: 10 additions & 3 deletions src/components/header/MenuOverlay.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,18 @@ class MenuOverlay extends Component {
})
})
}else {
breadcrumbRequest(nodeId).then(response => {
if (this.props.node) {
this.setState({
data: response.data
data: this.props.node
})
})
}
else {
breadcrumbRequest(nodeId).then(response => {
this.setState({
data: response.data
})
})
}
}

}
Expand Down