department-of-veterans-affairs / va.gov-cms

Editor-centered management for Veteran-centered content.
https://prod.cms.va.gov
GNU General Public License v2.0
97 stars 69 forks source link

Front End: Update VAMC appointment information in Service Location #16144

Closed xiongjaneg closed 6 months ago

xiongjaneg commented 10 months ago

User Story or Problem Statement

VAMC Facility Service appointment information will be migrated into updated Service Location in #15559. This aligns with the VBA appointments information being in Service Location. This will result in changes to how this information is displayed in accordions.

Changes to Drupal for service locations is still are still in this PR awaiting change management to VAMC

This needs to merge a day after #15559 makes it to prod to prevent editorial confusion.

Acceptance Criteria

Team

Please check the team(s) that will do this work.

xiongjaneg commented 10 months ago

Already have a button styling ticket #15601

Queries have some overlap, so keep in one ticket and keep the commits separate.

It doesn't have to be one PR but does have to be one integration branch.

mmiddaugh commented 9 months ago

@xiongjaneg I'd like to see us implement the full design with VAMC health services because we know it is challenging to scan and parse the information as separate service locations in the current state.

But let's talk through this issue during refinement

swirtSJW commented 7 months ago

Data will be coming from this tugboat instance https://pr15622-xptmfsnjoodldn85nzgkrgljqtfbz24b.ci.cms.va.gov/

eselkin commented 7 months ago

Just putting this here in case I lose the location of the Health Care Local Facility GraphQL src/site/stages/build/drupal/graphql/healthCareLocalFacilityPage.graphql.js

All the following fields are brought in in: src/site/stages/build/drupal/graphql/file-fragments/appointmentItems.graphql.js

However one field is missing: fieldReferralRequired Are we also removing this @xiongjaneg

eselkin commented 7 months ago

@swirtSJW Having trouble finding where the service location paragraph type is housed.

I see on NodeHealthCareLocalFacility there's a LocationService which can be a ParagraphServiceLocation, but is that the thing we're looking for?

Or is it connected some other way? I'm seeing empty ParagraphServiceLocaiton (no matching elements) for any of the Local Facilities I've searched through.

eselkin commented 7 months ago

@xiongjaneg Are we mimicking the design from the VBA accordions or keeping the design like the service locations paragraphs that exist already in VAMCs? Are we adding the icons for the walkins etc (I don't think we have them for VBA yet)? Are we using the components like the online scheduling active links etc (nvm I see we have another ticket for this, but it's kind of involved with this ticket since the source of that button's data is changing)? I see @mmiddaugh's comment from December was not commented on. I'm ok pulling in the design, just want to be sure we're ok with the change (including the cards, etc)

xiongjaneg commented 7 months ago

@eselkin Apologies, I forgot to link to Jordan's recently completed design for VAMC service location

xiongjaneg commented 7 months ago

From Steve: These should all be the same service location partial / component. That would cover VBA, non clinical services and VAMC service.

xiongjaneg commented 7 months ago

From Eli: There is some test in VAMC that doesn't render the entire paragraph, which is different from VBA. We can unify these but need to look at the conditionals that are currently used in VAMC service location paragraph and potentially copy in the VBA paragraph. We just broke out the VBA service location paragraphs so we can look at merging the two service location templates.

We don't have sample VBA data so we weren't exactly sure what was generated and what it would look like. We will need to generate that data for the service location migration.

xiongjaneg commented 7 months ago

@xiongjaneg stub ticket for other work outside of the scope of the ACs in this ticket.

davidmpickett commented 7 months ago

@eselkin I added some notes on your PR, but not sure what parts can be determined there vs what need to be split off into other tickets.

xiongjaneg commented 7 months ago

In progress. The challenge is that it's using a service location paragraph liquid template shared between VBA and VAMC. The problem is that they don't both align 100% on what they have and the spacing as a result of this between the two of them messes up the appearance.

jilladams commented 7 months ago

@eselkin can you verify status for end of sprint,as I missed the Service Location reckoning meeting yesterday? From PR it looks like for end of sprint: have addressed many pieces of design feedback, and I saw one remaining open question for @davidmpickett , but not sure if that's everything.

The one open question for Dave looked like:

Q:

This isn't good, but I don't know how to fix it If an editor has entered building name/number or wing, floor or room number, but not Service location address this information now will be floating around without context. Vs on Prod, this information would default to falling under the generic Contact Info header

Proposed Answer: Right now if there's no clinic name but there is subsequent data, we could default to something like: Location Info In the header space for that section.

eselkin commented 7 months ago

Yes. That is an open question about not having a header for the location @jilladams thanks for parsing through this long thread.

Just today @laflannery caught another issue with one of the nonclinical service pages. I fixed it and am waiting for Laura to review tomorrow.

It passed design review as of the changes to the VBA accordions.

davidmpickett commented 7 months ago

Q:

This isn't good, but I don't know how to fix it If an editor has entered building name/number or wing, floor or room number, but not Service location address this information now will be floating around without context. Vs on Prod, this information would default to falling under the generic Contact Info header

Proposed Answer: Right now if there's no clinic name but there is subsequent data, we could default to something like: Location Info In the header space for that section.

Might make sense to migrate this open question to @thejordanwood's new ticket for reviewing the design of non-clinical services. https://github.com/department-of-veterans-affairs/va.gov-cms/issues/17342

Or somewhere else in the new epic https://github.com/department-of-veterans-affairs/va.gov-cms/issues/17350

laflannery commented 7 months ago

@eselkin all the headings levels look good, I have no more comments or changes, this is approved by me.

Also, I'm adding all of the heading documentation I can possible add for sanity and for future reference and for all the things -

I reviewed the below pages and confirmed they all have proper headings levels regarding this new service location/situation display:

jilladams commented 7 months ago

@eselkin a flag here: ACs say " This ticket need to go into their own integration branch." but I see that your open PR is targeting main. Do we need to change that? Or are there reasons you've found it's ok to go ahead to main?

jilladams commented 7 months ago

@eselkin also: In sprint planning, we opted to take the DaveP Location Info question into #17342, so you can exclude that from your current PR. Which might mean that addressing target branch is the last blocker to getting this out the door for you?

jilladams commented 7 months ago

From planning: we noted this PR is the integration branch. (🤦‍♀️ ) So: work is complete now, ticket is pending integration when Drupal work is ready to merge.