fix n+1 problem for zoom account availability #82
Conversation
prabalsingh24
force-pushed
the
zoom-account-n+1
branch
2 times, most recently
from
a2b6ade to
ea4ff45
Compare
|
@prabalsingh24 you need to add separate loader for Resource.availability. |
api/routes/graphql/index.js
Outdated
| loaders: { | ||
| zoomAccountAvailabilityLoader: ZoomAccount.getAvailabilityLoader(), | ||
| }, |
There was a problem hiding this comment.
or do you mean to add a different loader here.
There was a problem hiding this comment.
zoomAccountResourceLoader something like this?
api/services/ZoomAccount/index.js
Outdated
| }, | ||
| ...utilizedResourceClause(args.dateTimeRange, args.options), | ||
| }).then((accounts) => | ||
| keys.map((key) => !accounts.find((account) => key.id === account.id)), |
There was a problem hiding this comment.
can't compare just based on id. Need to do deep equate for the whole key.
There was a problem hiding this comment.
There was a problem hiding this comment.
key : {
id: 28,
dateTimeRange: {
start_at: '2020-05-20T18:30:00.000Z',
end_at: '2020-05-20T19:30:00.000Z'
}
}
zoomAccount : {
id: 28,
email: '[email protected]',
created_at: 2020-05-21T06:45:50.203Z,
updated_at: 2020-05-21T06:45:50.203Z
}
There was a problem hiding this comment.
I don't think we can do deep equate here?
There was a problem hiding this comment.
You also get ZoomAccount.resource having start_at and end_at. Form the same shape with the data from db and then equate them.
api/services/ZoomAccount/index.js
Outdated
| ifAvailableDuring(dateTimeRange, options) { | ||
| return ZoomAccount.findAvailaibilityDuring( | ||
| this.instance.id, | ||
| dateTimeRange, | ||
| options, | ||
| ); |
There was a problem hiding this comment.
No need to change this.
|
Please update your branch with master. |
prabalsingh24
force-pushed
the
zoom-account-n+1
branch
from
f53a19b to
8881a31
Compare
|
I have few doubts. I've replied in above comments :) |
api/services/ZoomAccount/index.js
Outdated
| }, | ||
| ...utilizedResourceClause(args.dateTimeRange, args.options), | ||
| }).then((accounts) => | ||
| keys.map((key) => !accounts.find((account) => key.id === account.id)), |
There was a problem hiding this comment.
You also get ZoomAccount.resource having start_at and end_at. Form the same shape with the data from db and then equate them.
|
@prabalsingh24 what is the status on this? |
Sir can we discuss this over a call once? |
prabalsingh24
force-pushed
the
zoom-account-n+1
branch
from
e4aa6ad to
c2f31c0
Compare
prabalsingh24
force-pushed
the
zoom-account-n+1
branch
from
75974dd to
2e4bc08
Compare
|
I've made the changes and it is working too. Have a look whenever you're free :) Sorry I was little slow in doing this, I had my end semester exams |
| ); | ||
| }; | ||
|
|
||
| equateKeyAndInvite({ id, excludedEvents = [], dateTimeRange }, account) { |
There was a problem hiding this comment.
You are not equating key to invite here. Fix the name.
|
Looks good. Just minor change request. |
|
Congratualtions @prabalsingh24, your pull request is merged! 🎉 Thanks for your contributions and participating in BOSS 2020. 🙌 You can claim your bounty points here. 💰 |
fixes #18