Open vikasrohit opened 5 years ago
@maxceem One improvement we can do here is to don't make separate calls to loadMembers action for every phase topic load, instead of that we can make a combined call to loadMembers
after all phase topics are loaded successfully. Do you think that would create any issue in terms of rendering of the phase topics?
@vikasrohit I guess it should be ok. We will need to make sure that until members are loaded we can still display topics/posts in the phases with some placeholder info for users.
@maxceem I tried something here https://github.com/appirio-tech/connect-app/commit/a0d137b9dd2eaf7d7d94d383843f91de273b50e9, in case you want to take a look tomorrow morning.
@maxceem Another possible optimization would be to get phase topics in single call by filtering at referenceId
. I think we did have a rationale for fetching them individually but I am not able to recall. So, it might be a good time to refresh that.
@vikasrohit I guess we can do this optimization together with implementing a new related feature: https://github.com/appirio-tech/connect-app/issues/2835
Yes, absolutely.
@vikasrohit At the moment posts are not shown at the Phases discussion tabs on DEV, see https://connect.topcoder-dev.com/projects/7489/plan:
Could it be connected with this PR? https://github.com/appirio-tech/connect-app/pull/2840
Fully probable. Good catch. Do you think you would be able to look at it?
I think it fetching the posts for the phase discussions correctly, only thing is that the phase card is not picking up those discussions.
I can run a small F2F for this if you want. Too much things to do.
No worries. I can debug it today.
@maxceem I have patched the code for the issue. Let me know if you are still seeing any issue.
Works good to me!
@vikasrohit there is some issue connected with changes done in this commit https://github.com/appirio-tech/connect-app/commit/4f5b5800872f1a910090b86d73b6614a224156cb
When we load project http://local.topcoder-dev.com:3000/projects/7442/plan there is an error in console:
The reason for this error is that one of the server requests doesn't return topics for one of the phases:
After that the value
here is null
:
So then topics
array contains null
here:
I've made a fix in this commit https://github.com/appirio-tech/connect-app/commit/b602fd4b700cfe408fd0286f92d6ddfbcedd76fb. Just wanted to let you know in case you see some other root of this issue which should be fixed instead of doing such fix.
Thanks for catching that @maxceem.
Could we do _.get(resp, 'value', [])
instead of _.get(resp, 'value') ? [_.get(resp, 'value')] : []
?
Update: I think resp
has value
key with null
value, so it _.get
won't return default value.
We have to return an array here while value
is one element of the array. So if value
is defined _.get(resp, 'value', [])
would be one element instead of array.
As of now, we are making too many calls to fetch the data of the project on the dashboard and the reason is that we need to show some portion of every possible data attached with a project, on the dashboard. Along the feature development across releases, we introduced new API calls to fulfil certain requirements. However, it seems that we are now making some extra calls to the APIs which we might be able to reduce still keeping all the features working. Purpose of this tasks is to identify all such places where we can improve the page load performance, either by directly optimizing the code of the front end or identify the API changes which can significantly improve the performance.
fyi @maxceem @RishiRajSahu