Fix: slow consultation pages#66
Open
LiamStanziani wants to merge 14 commits intoMagentaHealth:release/2026-01-19from
Open
Fix: slow consultation pages#66LiamStanziani wants to merge 14 commits intoMagentaHealth:release/2026-01-19from
LiamStanziani wants to merge 14 commits intoMagentaHealth:release/2026-01-19from
Conversation
Reviewer's GuideRefactors consultation request listing and form utilities to use DTO-based JPQL projections, batched loading of extensions, and new DAO methods while adding targeted database indexes and query hints to significantly improve consultation page performance and robustness. Sequence diagram for consultation list loading with DTO and batched extensionssequenceDiagram
actor User
participant JSP as ConsultationListJSP
participant Util as EctViewConsultationRequestsUtil
participant CRDao as ConsultationRequestDao
participant CRDaoImpl as ConsultationRequestDaoImpl
participant DB as Database
User->>JSP: Request consultation list page
JSP->>Util: estConsultationVecByTeam(loggedInInfo, team,...)
Util->>CRDao: getConsultationDTOs(team, showCompleted, startDate, endDate, orderby, desc, searchDate, offset, limit)
activate CRDao
CRDao->>CRDaoImpl: getConsultationDTOs(...)
activate CRDaoImpl
note over CRDaoImpl: Build JPQL DTO_SELECT + DTO_FROM + filters
CRDaoImpl->>DB: JPQL query
Note right of DB: Single constructor projection
Note right of DB: LEFT JOIN Demographic, Provider, ConsultationServices, ProfessionalSpecialist
DB-->>CRDaoImpl: List<ConsultationListDTO>
note over CRDaoImpl: loadExtensionsForDTOs(dtos)
CRDaoImpl->>DB: SELECT e FROM ConsultationRequestExt e WHERE e.requestId IN (:ids)
DB-->>CRDaoImpl: List<ConsultationRequestExt>
note over CRDaoImpl: Group by requestId and dto.applyExtensions(extMap)
CRDaoImpl-->>CRDao: List<ConsultationListDTO>
deactivate CRDaoImpl
CRDao-->>Util: List<ConsultationListDTO>
deactivate CRDao
note over Util: initLists()
note over Util: For each dto, populate parallel lists
Util-->>JSP: Populated lists (ids, status, patient, ...)
JSP-->>User: Rendered consultation list page
ER diagram for consultationRequests and new performance indexeserDiagram
consultationRequests {
int id PK
varchar status
int demographicNo FK
varchar providerNo FK
varchar sendTo
int serviceId FK
int specId FK
date referalDate
date appointmentDate
date followUpDate
datetime lastUpdateDate
%% Indexed columns
%% idx_consult_status_referaldate (status, referalDate)
%% idx_consult_sendto_status_referaldate (sendTo, status, referalDate)
%% idx_consult_providerno_status_referaldate (providerNo, status, referalDate)
%% idx_consult_status_appointmentdate (status, appointmentDate)
%% idx_consult_demographicno (demographicNo)
%% idx_consult_serviceid (serviceId)
%% idx_consult_specid (specId)
%% idx_consult_lastupdatedate (lastUpdateDate)
}
Demographic {
int DemographicNo PK
varchar LastName
varchar FirstName
varchar ProviderNo FK
}
Provider {
varchar ProviderNo PK
varchar LastName
varchar FirstName
}
ConsultationServices {
int serviceId PK
varchar serviceDesc
}
ProfessionalSpecialist {
int specId PK
varchar firstName
varchar lastName
}
ConsultationRequestExt {
int id PK
int requestId FK
varchar key
varchar value
}
%% Relationships used by DTO and optimized queries
consultationRequests ||--o{ ConsultationRequestExt : has
consultationRequests }o--|| Demographic : demographicNo
consultationRequests }o--|| Provider : providerNo
Demographic }o--|| Provider : mrpProvider
consultationRequests }o--|| ConsultationServices : serviceId
consultationRequests }o--|| ProfessionalSpecialist : specId
Updated class diagram for consultation request DTOs, DAOs, and utilitiesclassDiagram
class EctViewConsultationRequestsUtil {
List~String~ ids
List~String~ status
List~String~ patient
List~String~ provider
List~String~ teams
List~String~ service
List~String~ vSpecialist
List~String~ date
List~String~ demographicNo
List~String~ apptDate
List~String~ patientWillBook
List~String~ urgency
List~String~ followUpDate
List~String~ providerNo
List~String~ siteName
List~Provider~ consultProvider
List~Boolean~ eReferral
+boolean estConsultationVecByTeam(loggedInInfo, team)
+boolean estConsultationVecByTeam(loggedInInfo, team, showCompleted)
+boolean estConsultationVecByTeam(loggedInInfo, team, showCompleted, startDate, endDate)
+boolean estConsultationVecByTeam(loggedInInfo, team, showCompleted, startDate, endDate, orderby)
+boolean estConsultationVecByTeam(loggedInInfo, team, showCompleted, startDate, endDate, orderby, desc)
+boolean estConsultationVecByTeam(loggedInInfo, team, showCompleted, startDate, endDate, orderby, desc, searchDate, offset, limit)
+boolean estConsultationVecByDemographic(loggedInInfo, demoNo)
-Provider buildConsultProvider(dto)
-boolean isBlank(value)
-void initLists()
}
class ConsultationRequestDao {
+List~ConsultationRequest~ getConsults(team, showCompleted, startDate, endDate, orderby, desc, searchDate, offset, limit)
+List~ConsultationRequest~ getConsults(demoNo)
+List~ConsultationListDTO~ getConsultationDTOs(team, showCompleted, startDate, endDate, orderby, desc, searchDate, offset, limit)
+List~ConsultationListDTO~ getConsultationDTOsByDemographic(demoNo)
}
class ConsultationRequestDaoImpl {
-static String DTO_SELECT
-static String DTO_FROM
+List~ConsultationListDTO~ getConsultationDTOs(team, showCompleted, startDate, endDate, orderby, desc, searchDate, offset, limit)
+List~ConsultationListDTO~ getConsultationDTOsByDemographic(demoNo)
-String buildConsultationDTOQuery(paramList, team, showCompleted, startDate, endDate, orderby, desc, searchDate)
-void loadExtensionsForDTOs(dtos)
}
class ConsultationListDTO {
-static long serialVersionUID
-static String NOT_APPLICABLE
-Integer id
-String status
-String urgency
-Integer demographicNo
-String demographicLastName
-String demographicFirstName
-String demographicProviderNo
-String mrpLastName
-String mrpFirstName
-String consultProviderNo
-String consultProviderLastName
-String consultProviderFirstName
-Integer serviceId
-String serviceDescription
-String specialistLastName
-String specialistFirstName
-Date referralDate
-Date appointmentDate
-Date appointmentTime
-boolean patientWillBook
-Date followUpDate
-String sendTo
-String siteName
-boolean eReferral
-String ereferralService
-String ereferralDoctor
+ConsultationListDTO()
+ConsultationListDTO(id, status, urgency, demographicNo, demographicLastName, demographicFirstName, demographicProviderNo, mrpLastName, mrpFirstName, consultProviderNo, consultProviderLastName, consultProviderFirstName, serviceId, serviceDescription, specialistLastName, specialistFirstName, referralDate, appointmentDate, appointmentTime, patientWillBook, followUpDate, sendTo, siteName)
+String getPatientFormattedName()
+String getMrpFormattedName()
+String getConsultProviderFormattedName()
+String getSpecialistFormattedName()
+String getEffectiveServiceDescription()
+String getReferralDateFormatted()
+String getAppointmentDateFormatted()
+String getFollowUpDateFormatted()
+void applyExtensions(extMap)
+Integer getId()
+String getStatus()
+String getUrgency()
+Integer getDemographicNo()
+String getDemographicProviderNo()
+String getConsultProviderNo()
+String getConsultProviderLastName()
+String getConsultProviderFirstName()
+Integer getServiceId()
+String getServiceDescription()
+Date getReferralDate()
+Date getAppointmentDate()
+Date getAppointmentTime()
+boolean isPatientWillBook()
+Date getFollowUpDate()
+String getSendTo()
+String getSiteName()
+boolean isEReferral()
}
class ConsultationRequest {
+Integer id
+String status
+Integer demographicId
+Integer serviceId
+Integer specId
+String providerNo
+Date referralDate
+Date appointmentDate
+Date appointmentTime
+Date followUpDate
+String sendTo
+String siteName
+boolean patientWillBook
+Date lastUpdateDate
+ProfessionalSpecialist professionalSpecialist
+DemographicContact demographicContact
+LookupListItem lookupListItem
}
class ProfessionalSpecialist {
+Integer specId
+String firstName
+String lastName
+String phoneNumber
+String faxNumber
+String streetAddress
+String emailAddress
}
class DemographicContact {
+Integer id
}
class LookupListItem {
+String value
}
class ConsultationServiceDao {
+ConsultationServices find(serviceId)
+ConsultationServices findByDescription(description)
+ConsultationServices findReferringDoctorService(activeOnly)
+String getServiceDescription(serviceId)
+List~ConsultationServiceDto~ findActiveServiceSummaries()
}
class ConsultationServiceDaoImpl {
+List~ConsultationServiceDto~ findActiveServiceSummaries()
+String getServiceDescription(serviceId)
}
class ConsultationServiceDto {
+Integer serviceId
+String serviceDesc
}
class FaxJobDao {
+List~FaxJob~ getFaxStatusByDateDemographicProviderStatusTeam(demographicNo, providerNo, status, team, startDate, endDate)
+List~FaxJob~ getInprogressFaxesByJobId()
+List~FaxJob~ findByIds(ids)
}
class FaxJobDaoImpl {
+List~FaxJob~ findByIds(ids)
}
class FaxJob {
+Integer id
}
class EctConsultationFormRequestUtil {
-ProfessionalSpecialist professionalSpecialist
-String specPhone
-String specFax
-String specAddr
-String specEmail
-boolean isEReferral
+boolean estPatient(loggedInInfo, demographicNo)
+boolean estRequestFromId(loggedInInfo, id)
-void getFaxLogs(requestId)
+String getServiceName(id)
}
class ConsultationRequestExt {
+Integer id
+Integer requestId
+String key
+String value
}
%% Relationships
ConsultationRequestDaoImpl ..|> ConsultationRequestDao
ConsultationRequestDaoImpl --> ConsultationListDTO
ConsultationRequestDaoImpl --> ConsultationRequestExt
ConsultationRequestDaoImpl --> ConsultationRequest
EctViewConsultationRequestsUtil --> ConsultationRequestDao
EctViewConsultationRequestsUtil --> ConsultationListDTO
EctViewConsultationRequestsUtil --> Provider
ConsultationRequest --> ProfessionalSpecialist
ConsultationRequest --> DemographicContact
ConsultationRequest --> LookupListItem
ConsultationServiceDaoImpl ..|> ConsultationServiceDao
ConsultationServiceDaoImpl --> ConsultationServices
ConsultationServiceDaoImpl --> ConsultationServiceDto
FaxJobDaoImpl ..|> FaxJobDao
EctConsultationFormRequestUtil --> ConsultationRequestDao
EctConsultationFormRequestUtil --> ConsultationRequestExt
EctConsultationFormRequestUtil --> FaxJobDao
EctConsultationFormRequestUtil --> FaxJob
EctConsultationFormRequestUtil --> ProfessionalSpecialist
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
@Fetch(FetchMode.SELECT)annotations on eagerly loaded associations may still cause N+1 query patterns; consider switching these relationships toLAZYwhere possible and explicitly fetching them in the queries that need them. - The migration script uses
CREATE INDEX IF NOT EXISTS, which is only supported in newer MySQL versions; verify that all supported environments run a compatible version or adjust the script to be backward compatible.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `@Fetch(FetchMode.SELECT)` annotations on eagerly loaded associations may still cause N+1 query patterns; consider switching these relationships to `LAZY` where possible and explicitly fetching them in the queries that need them.
- The migration script uses `CREATE INDEX IF NOT EXISTS`, which is only supported in newer MySQL versions; verify that all supported environments run a compatible version or adjust the script to be backward compatible.
## Individual Comments
### Comment 1
<location> `database/mysql/updates/update-2026-01-26-consultation-indexes.sql:5` </location>
<code_context>
+-- Addresses slow consultation page load times by optimizing common query patterns
+
+-- Index for status + referral date filtering (common filter pattern)
+CREATE INDEX IF NOT EXISTS idx_consult_status_referaldate ON consultationRequests (status, referalDate);
+
+-- Index for team-based queries with status and date filtering (main team listing query)
</code_context>
<issue_to_address>
**issue (bug_risk):** MySQL does not support `CREATE INDEX IF NOT EXISTS`, which will cause this migration to fail.
These statements will raise a syntax error on MySQL. If this migration must support MySQL, remove `IF NOT EXISTS` and either rely on the migration framework’s one-time execution guarantees or add a separate, framework-compatible existence check (e.g., via `information_schema`) before creating the index.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Collaborator
Author
|
Depending on if the tickler issue is still happening on prod, I might need to change the solution, and create a new solution for the tickler updates, will set to draft until further investigation is conducted on the state of that in prod |
Collaborator
Author
|
I am going to revisit this after I fix up the meds disappearing issue, I think the main problem is probably:
|
…o speed up queries more
…tch case instead of if else for performance, remove space seperator from date time string
…void potential NullPointerExceptions, add NumberFormatException handling inside of EctConsultationFormRequestUtil and ctViewConsultationRequestsUtil, prevent stale professionalSpecialist values by clearing contact fields if request is null, restore legacy date time matching, sanitize field before audit logging, added javadoc to FaxJobDao and FaxJobDaoImpl new methods
…rvices entities and their EAGER specialist collections
…alize blank providerNo's to ensure rows load correctly with privacy filters, return null provider to avoid outputting null provider name, populate all parallel lists in ectConsultationVecByDemographic to preven IndexOutOfBounds Exception, replace hardcoded extension keys for enum refs, add extra context to error log messages, return default sorting by ascending
LiamStanziani
force-pushed
the
bug/slow-consult-pages
branch
from
8d71af4 to
0eba7d2
Compare
Collaborator
Author
|
Force pushed to amend commit message with some fixes to a typo, and one missing fix explanation to it |
Collaborator
Author
|
This PR is looking good at this point, I will see if there are any last AI comments with this PR through sourcery and then it will be ready for review |
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In ConsultationRequestDaoImpl, DTO_SELECT/DTO_FROM and buildConsultationDTOQuery are built with table/column-style identifiers (e.g., Demographic d, d.DemographicNo, ON clauses) but are executed via createQuery as JPQL; this will not parse as JPQL, so either switch to createNativeQuery with proper result mapping or refactor the query to valid JPQL using entity field names and join syntax.
- The Javadoc for ConsultationRequestDao#getConsultationDTOsByDemographic states results are ordered by referral date descending, but the implementation in ConsultationRequestDaoImpl uses ORDER BY cr.referralDate ASC; align the ordering with the documented behavior or update the Javadoc to avoid surprises.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In ConsultationRequestDaoImpl, DTO_SELECT/DTO_FROM and buildConsultationDTOQuery are built with table/column-style identifiers (e.g., Demographic d, d.DemographicNo, ON clauses) but are executed via createQuery as JPQL; this will not parse as JPQL, so either switch to createNativeQuery with proper result mapping or refactor the query to valid JPQL using entity field names and join syntax.
- The Javadoc for ConsultationRequestDao#getConsultationDTOsByDemographic states results are ordered by referral date descending, but the implementation in ConsultationRequestDaoImpl uses ORDER BY cr.referralDate ASC; align the ordering with the documented behavior or update the Javadoc to avoid surprises.
## Individual Comments
### Comment 1
<location> `src/main/java/ca/openosp/openo/commn/dao/ConsultationRequestDaoImpl.java:110-119` </location>
<code_context>
}
}
- String orderDesc = desc != null && desc.equals("1") ? "DESC" : "";
- String service = ", service.serviceDesc";
if (orderby == null) {
</code_context>
<issue_to_address>
**issue (bug_risk):** ORDER BY clauses reference JPQL fields with database-style casing, which likely do not match entity property names.
In `buildConsultationDTOQuery`, the `orderby` switch uses `d.LastName`, `mrp.LastName`, and `svc.serviceDesc`. JPQL is case-sensitive and requires entity property names (e.g. `d.lastName`), not column names. Update these ORDER BY expressions to use the correct entity properties, consistent with other JPQL in the project, to avoid query failures.
</issue_to_address>
### Comment 2
<location> `src/main/java/ca/openosp/openo/commn/dao/ConsultationRequestDaoImpl.java:279-280` </location>
<code_context>
+ */
+ @Override
+ public List<ConsultationListDTO> getConsultationDTOsByDemographic(Integer demoNo) {
+ String sql = DTO_SELECT + DTO_FROM +
+ "WHERE cr.demographicId = ?1 " +
+ "ORDER BY cr.referralDate ASC";
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Method-level documentation promises descending order but the implementation orders by referral date ascending.
The Javadoc for `getConsultationDTOsByDemographic` specifies "referral date descending", but the query uses `ORDER BY cr.referralDate ASC`. Please align the query and documentation so callers get the expected sort order.
</issue_to_address>
### Comment 3
<location> `database/mysql/updates/update-2026-01-26-consultation-indexes.sql:5` </location>
<code_context>
+-- Addresses slow consultation page load times by optimizing common query patterns
+
+-- Index for status + referral date filtering (common filter pattern)
+CREATE INDEX IF NOT EXISTS idx_consult_status_referaldate ON consultationRequests (status, referalDate);
+
+-- Index for team-based queries with status and date filtering (main team listing query)
</code_context>
<issue_to_address>
**issue (bug_risk):** Index definition appears to use a misspelled column name (`referalDate`) which may not match the actual schema.
The entity and queries use `referralDate`, but this index targets `referalDate`. If the actual column is `referralDate`, this migration will fail at deploy. Please confirm the correct column name and align both the index name and column reference accordingly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
src/main/java/ca/openosp/openo/commn/dao/ConsultationRequestDaoImpl.java
Show resolved
Hide resolved
src/main/java/ca/openosp/openo/commn/dao/ConsultationRequestDaoImpl.java
Show resolved
Hide resolved
Collaborator
Author
|
Ready for review @lacarmen TIcket link for manual testing: openo-beta#2099 |
Collaborator
Author
|
Deval has reviewed this PR, see this comment from the other PR (openo-beta#2264) 
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please run these indexes as well:
In this PR, I have fixed:
I have tested these by:
Summary by Sourcery
Optimize consultation request listing and form loading for performance and robustness by introducing DTO-based queries, batch-loading related data and extensions, and adding targeted database indexes.
Enhancements:
Build:
Documentation: