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

Public resources for building on and in support of VA.gov. Visit complete Knowledge Hub:
https://depo-platform-documentation.scrollhelp.site/index.html
283 stars 204 forks source link

Design | Profile | Military Info | Documentation #45856

Closed SKasimow closed 2 years ago

SKasimow commented 2 years ago

Background

Audit and update the Military info documentation in Github.

Tasks

Create the following

Update the following

Acceptance Criteria

SKasimow commented 2 years ago

Hi @andaleliz - adding this in for Sprint 81.

andaleliz commented 2 years ago

@adamwhitlock1 I'm trying to figure out what alerts we have for military information. In the Sketch file, I see a bunch of different alerts that are really similar. I'm not sure which ones are in use and it's not clear to me what the distinct use case would be for each.

I'm also not sure if the additional info is visible on the page when all we show is an alert

Are you able to look into the code and tell me which of these are actually in use, and if the additional info component is visible? Hopefully the link I pasted works, but here are screen shots in case it doesn't:

image image image

Please and thank you in advance for any help!

andaleliz commented 2 years ago

@Samara-Strauss are there any analytics you track for this page?

I found the page data report in GA, not sure if that's helpful to include in the product outline. There's a note there about adding GET/POST data from GA.

Samara-Strauss commented 2 years ago

Yes -- we just added a module to this dashboard to track military information GETs https://analytics.google.com/analytics/web/?authuser=0#/dashboard/-x0K5pQPRTaQCa_WzXnEDg/a50123418w177519031p176188361/

SKasimow commented 2 years ago

@andaleliz - while doing the internal UAT for the Military Info migration project, we ran into a user who attended a military academy but didn't go into service. So he appears as "authorized" and sees this message: image (34)

I on the other hand, don't have either a military academy or service in my background so I am "unauthorized" and see this message: Screen Shot 2022-08-02 at 1 27 02 PM

andaleliz commented 2 years ago

Thanks @SKasimow! That's helpful. Noting our discussion in stand-up here: Adam will check to see if he can learn more about any other use cases these might apply to by looking at the code 👀

@SKasimow I notice that both of these alerts don't follow our current content guidelines, and some tweaks are needed. For example, in the first one - we don't provide any info about how a person could file that request mentioned.

Can you please create a ticket for design to bring these to content office hours, and then update the Sketch files? Either one of us can tackle this when we have time. Thank you!

andaleliz commented 2 years ago

@Samara-Strauss I've got a few questions you may be able to answer:

  1. In the profile staging test accounts doc, the 3rd test case contradicts itself. It says "IS NOT unavailable in DEERS". I'm guessing it should say "IS NOT available" - am I right?

  2. Do we need to keep the 2nd and 4th use cases separate? They seem duplicative.

  3. We have to separate places where we are keeping military info staging user info - the overall profile doc, and the one for the recent work. Can they be combined? IMO it's nice to have a dedicated file for each feature rather than all in one big profile doc.

  4. Do you have any insights on my above question about alert use cases?

Thank you!

Samara-Strauss commented 2 years ago
  1. Yes, that is likely a typo. But I should also mention the users who did or did not have military information before are not necessarily going to be the same now since we switched our integration from eMIS to VA Profile.
  2. Yes, they are the same.
  3. I deleted the doc for the recent work since all of those users did not actually have military info in VA Profile. Some did, some didn't.
  4. Sharon's post answered your question -- there is a difference between what we show to people for who aren't veterans and have no military history vs. people who are but we can't retrieve their military history at the moment. As for the "please refresh the page" error, I am not sure if that one actually exists or not.
andaleliz commented 2 years ago

Thanks @Samara-Strauss ! That answers most of my questions. @adamwhitlock1 I guess my main question for you now is do we show that "We can't load your military information" error at any point?

According to the call center docs, we show the "We can't access..." message when we have connected to DEERS but there isn't a DoD ID associated with their record, and it's not clear to me if we're also using this same alert when we can't connect to DEERS at all.

IMO we should be using a different alert when we can't connect to DEERS at all, something to the effect of "This page isn't available right now, check back later" like we do in other parts of profile.

andaleliz commented 2 years ago

Hi @SKasimow and @Samara-Strauss, I've completed the documentation update. The docs are ready for your review. This one is fairly straightforward since it's a read-only feature.

Samara-Strauss commented 2 years ago

Outline

Use cases overview

Military information: user wants to access their military information

adamwhitlock1 commented 2 years ago

@andaleliz I took a bit of a deep dive into the alerts and think I have figured out exactly what is showing.

I don't believe that this alert design will ever show, as it isn't in the code anywhere (sketch file is labeled as Military Information - alert 1):

Screen Shot 2022-08-23 at 8 48 37 AM

Instead of the 'alert 1' when the info cant load, the file named Military Information 2 Copy 2 is shown, but it does not show the additional information expandable component.

Example:

Screen Shot 2022-08-23 at 8 52 24 AM

In regards to our conversation about the "NotInDEERSAlert" I don't think that alert is ever being shown and instead is showing the more general alert (sketch file called alert 3) for if they are not found in DEERS.

I checked this by using the test user vets.gov.user+10@gmail.com which according to our testing user docs is supposed to be a user explicitly not in DEERS. When looking at the state data of this user it lined up with the more generic '403' permission denied / not found data.


Some other minor things I noticed when evaluating our designs vs our code rendering:

Phone links in the design files are not styled like links in that they are not underlined and blue in text color like our implementation.

CODE: Screen Shot 2022-08-23 at 9 14 13 AM

DESIGN FILE:

Screen Shot 2022-08-23 at 9 14 21 AM

For the additional information component "What if my military service information doesn't look right?" I noticed that the code uses an expandable section without a side border and the design includes the border:

CODE: Screen Shot 2022-08-23 at 8 56 18 AM

DESIGN:

Screen Shot 2022-08-23 at 9 18 58 AM

I hope this helps sort out what is in use and what is not, and please let me know if you have anymore questions! Thanks for all the awesome work you have been putting into this documentation Liz.

andaleliz commented 2 years ago

@adamwhitlock1 thanks for digging into this! You uncovered some discrepancies I noticed too. It's so helpful to get another set of eyes on this.


In regards to our conversation about the "NotInDEERSAlert" I don't think that alert is ever being shown and instead is showing the more general alert (sketch file called alert 3) for if they are not found in DEERS.

I think I understand, but would appreciate it if you can confirm - essentially, the Sketch files called alert 2 is never shown. Is that correct?


Phone links in the design files are not styled like links in that they are not underlined and blue in text color like our implementation.

I updated the Sketch files; these should be links.


For the additional information component "What if my military service information doesn't look right?" I noticed that the code uses an expandable section without a side border and the design includes the border:

For this one, I think we need to add the blue border into the code. We use the border in all other parts of profile, and I think it was an oversight that it wasn't included here in the original build.

@SKasimow can you please create a FE ticket to have this updated?


I don't believe that this alert design will ever show, as it isn't in the code anywhere (sketch file is labeled as Military Information - alert 1):

That is correct; I updated this file yesterday to align with the other alert updates captured in #45872. I added this one to the list in that ticket.

andaleliz commented 2 years ago

Oh, and I'm going to rename the Sketch artboards so they're more helpful than "alert 2 copy 2" 😆

adamwhitlock1 commented 2 years ago

This is how the NotInDeersAlert renders if it ever showed, which I'm 99% sure won't ever show, and instead the doesn't have records will render.

Screen Shot 2022-08-23 at 10 30 07 AM ^ This design doesn't actually seem to be present in your designs, so if I can for certain validate that it shouldn't ever show, then I think I should remove the code for this alert altogether.

Alert 2 is still needed for an error resulting from a server error like a 500 error or other unknown system error.

I think that in reality the page unavailable design should be removed since I dont seen anything in the code that would align with that design.

Samara-Strauss commented 2 years ago

This is how the NotInDeersAlert renders if it ever showed, which I'm 99% sure won't ever show, and instead the doesn't have records will render.

I'm ok with removing the ☝️ NotInDeersAlert alert code if we're not actually using it.

I think that in reality the page unavailable design should be removed since I dont seen anything in the code that would align with that design.

Also OK with removing this from our documentation if we aren't actually using it in the code.

andaleliz commented 2 years ago

Adam and I met to discuss the alerts since there was so much back and forth. We are going to remove the NotInDeersAlert, and the page unavailable alert will be updated via the FE ticket I referenced a couple comments earlier.

The "we can't seem...." alert that Sharon referenced (screenshot below) is actually being used, and it needs a content update. @SKasimow can you please create a design ticket for a future sprint to get that aligned w/ our content style, and include more helpful information for people? It may take a bit of digging around because filing a request to change your DD214 isn't something VA handles. We'll need to do some research to figure out how to advise people. image

I think I'm clear on the rest of the alerts and have updated the use cases accordingly.

@adamwhitlock1 would you mind double checking my work please, before I call this done? I just want to make sure the info is accurate that is associated with the "we can't seem to find..." alert (doc) and the "we can't access..." alert (doc. I think the difference is in not having a DoD ID, and not having a service history, at least according to the call center documentation.

Thanks!

Samara-Strauss commented 2 years ago

All of this sounds good to me. Thanks for being so thorough, Liz!

SKasimow commented 2 years ago

@andaleliz - I created ticket #46149; should I add this to Sprint 81?

@SKasimow I notice that both of these alerts don't follow our current content guidelines, and some tweaks are needed. For example, in the first one - we don't provide any info about how a person could file that request mentioned.

Can you please create a ticket for design to bring these to content office hours, and then update the Sketch files? Either one of us can tackle this when we have time. Thank you!

SKasimow commented 2 years ago

@andaleliz - opened #46150 and placed in backlog:

For this one, I think we need to add the blue border into the code. We use the border in all other parts of profile, and I think it was an oversight that it wasn't included here in the original build.

@SKasimow can you please create a FE ticket to have this updated?

SKasimow commented 2 years ago

@andaleliz - is this complete?

andaleliz commented 2 years ago

@SKasimow I'm still waiting for @adamwhitlock1 to reply to my comment

andaleliz commented 2 years ago

@SKasimow RE your question about #46149 - up to you, if it's a priority for the sprint goals, or nothing else to take it's place. I will have bandwidth to work on it if it is something that'll be helpful 😄

adamwhitlock1 commented 2 years ago

@andaleliz Took a look and yes, those look correct! 🚀

andaleliz commented 2 years ago

Hooray, thank you! Closing this out.

SKasimow commented 2 years ago

Hi @andaleliz - great will add #46149 for you this sprint. There are a bunch of these UI/alert updates I'd love to have us knock out.