Skip to content

fix #337 #341

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 14, 2021
Merged

fix #337 #341

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cf7aeac
fix #337
xxcxy Jun 11, 2021
fb3bcc8
fix search WorkPeriod
xxcxy Jun 12, 2021
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
1 change: 1 addition & 0 deletions src/common/helper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,7 @@ function setResHeaders (req, res, result) {
res.set('X-Per-Page', result.perPage)
res.set('X-Total', result.total)
res.set('X-Total-Pages', totalPages)
res.set('X-Data-Source', result.fromDb ? 'database' : 'elasticsearch')
// set Link header
if (totalPages > 0) {
let link = `<${getPageLink(req, 1)}>; rel="first", <${getPageLink(
Expand Down
3 changes: 2 additions & 1 deletion src/services/InterviewService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -539,9 +539,10 @@ async function searchInterviews (currentUser, jobCandidateId, criteria) {
limit: perPage,
order: [[criteria.sortBy, criteria.sortOrder]]
})
const total = await Interview.count({ where: filter })
return {
fromDb: true,
total: interviews.length,
total,
page,
perPage,
result: _.map(interviews, interview => interview.dataValues)
Expand Down
3 changes: 2 additions & 1 deletion src/services/JobCandidateService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,9 +316,10 @@ async function searchJobCandidates (currentUser, criteria) {
limit: perPage,
order: [[criteria.sortBy, criteria.sortOrder]]
})
const total = await JobCandidate.count({ where: filter })
return {
fromDb: true,
total: jobCandidates.length,
total,
page,
perPage,
result: _.map(jobCandidates, jobCandidate => _.omit(jobCandidate.dataValues, omitList))
Expand Down
3 changes: 2 additions & 1 deletion src/services/JobService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,9 +532,10 @@ async function searchJobs (currentUser, criteria, options = { returnAll: false }
required: false
}]
})
const total = await Job.count({ where: filter })
return {
fromDb: true,
total: jobs.length,
total,
page,
perPage,
result: _.map(jobs, job => job.dataValues)
Expand Down
3 changes: 2 additions & 1 deletion src/services/WorkPeriodPaymentService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,9 +381,10 @@ async function searchWorkPeriodPayments (currentUser, criteria, options = { retu
limit: perPage,
order: [[criteria.sortBy, criteria.sortOrder]]
})
const total = await WorkPeriodPayment.count({ where: filter })
return {
fromDb: true,
total: workPeriodPayments.length,
total,
page,
perPage,
result: workPeriodPayments
Expand Down
5 changes: 3 additions & 2 deletions src/services/WorkPeriodService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,10 +519,11 @@ async function searchWorkPeriods (currentUser, criteria, options = { returnAll:
required: false
}]
}
const workPeriods = await WorkPeriod.findAll(queryCriteria)
const workPeriods = await WorkPeriod.findAndCountAll(queryCriteria)
const total = await WorkPeriod.count({ where: filter })
Copy link
Contributor

Choose a reason for hiding this comment

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

@xxcxy I didn't know such a method exists. I wonder why you didn't use the output from this method and call await WorkPeriod.count({ where: filter }) anyway below?

Can we just call findAndCountAll?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Strange, I forgot to change this. I tried findAndCountAll, the count returned by this method is wrong.

Copy link
Member

Choose a reason for hiding this comment

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

@maxceem @xxcxy
Fyi, findAndCountAll is safe when we don't have include option in query. Otherwise it gives wrong count number and becomes complex to solve.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @eisbilir. I think than it’s better not use it, because if we use it one place then new developer can copy paste it to a new place with include and would not notice that it works incorrectly there.

return {
fromDb: true,
total: workPeriods.length,
total,
page,
perPage,
result: workPeriods
Expand Down