-
Notifications
You must be signed in to change notification settings - Fork 69
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.
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
All published VBA services are not showing on the FE #18214
Comments
There should not a be a limit. All published services should display. This is a problem and should be investigated ASAP. While this might not technically be blocking of Staging Review (since it is unlikely for Platform to notice) it will be prevent our team from fully testing & QAing VBA. |
Eli verified: in the code there is no set limit. That makes the missing services more mysterious. |
Turns out Drupal sets a limit to 10 when no explicit limit is set on the subqueries. What is an appropriate limit we should set? 25? 50? 100? I'm guessing we should go with an arbitrarily highish (100) number since we always want to return all services. @Agile6MSkinner @jilladams |
Copying my notes from Slack BLUF: Other places where we are doing queries like this seem to use a limit of 500 (example 1, example 2). Similar to the epiphany I had in in 17681, this seems to be another reason why VAMCs and Vet Centers call Services as a Field Query rather than a Reverse Node Query. A field query doesn't seem to have a limit (unless the field itself has a cardinality limit). While adding a limit (here I think?) would be a quick fix, this makes me think that long term we should probably add an Entity Reference field to VBA Facilities to pull in the list of Services (Drupal Config PR) and switch the query to a Field Query (Content build PR). That would make the queries more consistent across facility types and also not having to declare a limit seems more future proof than just picking a high number. |
Updated my 100 limit to 500 per similar limits, though that's a combined total of 1000 services... Not that any facility will reach that, but that seems like a lot. |
@Agile6MSkinner We need some product help here to decide whether / how to modify the current Drupal implementation. Dave left notes above -- if we do want to align the handling of how we call services acros products, that's new Drupal work & FE template work we need to do to update this implementation. Could you make a note of this for us to talk about in our next refinement, to decide if this is done for now, or if we want to call this tech debt, and clean it up now vs. later? cc @mmiddaugh |
@jilladams Ok, I've moved this to the top of the order for next refinement. EDIT: @eselkin @davidmpickett That occurs on Wednesday. If we make a decision then, will that leave us enough time to complete the work by the end of sprint (Tues 6/25)? |
The work to do the limit increase is in PR (@dsasser is waiting on us resolving whether we want to go ahead wit the limit increase or do the field modifications and changes to graphql). If we change to do the graphql changes and field updates, I doubt that would be done before the end of sprint. |
This can't really be verified on prod because no VBAs are published and Preview Server won't show services. So instead I am doing a Content Release on my tugboat with Main & Main (whereas I previously was using PR 2128 for content-build. Once that completes should be verifiable here: https://web-jdujzwgvp8kmj36hcfonqejlgtq5b6st.demo.cms.va.gov/philadelphia-va-regional-benefit-office/ |
Description
Not all services are showing on the front end on the facilty page. It seems like there might be a max of 10 total set somewhere. For example:
Honolulu has 15 published and only 10 showing. The missing ones are and I noticed that those were the most recently published ones:
National Capital as 11 published and only 10 showing, again this is the last published one:
Acceptance Criteria
The text was updated successfully, but these errors were encountered: