VACMS-16290 Event Listing

[jtmst]

Description

Closes #16290.

Testing done

Tested a variety of event listing pages + the different filters on the event list widget
Tested Lovell event listing

Screenshots

Standard Event List:

Lovell:

Outreach and Events (no menu)

QA steps

Tasks

Preview Give feedback

1. Automated tests have passed
2. Tugboat environment was generated without errors
3. https://pr322-co7f0k99bqet9iujnhtae4twqyxjwfpi.tugboat.vfs.va.gov/butler-health-care/events/ renders as expected
4. https://pr322-co7f0k99bqet9iujnhtae4twqyxjwfpi.tugboat.vfs.va.gov/lovell-federal-health-care-va/events/ renders as expected and can be switched to Lovell TRICARE
5. https://pr322-co7f0k99bqet9iujnhtae4twqyxjwfpi.tugboat.vfs.va.gov/outreach-and-events/events/ renders as expected without a menu
6. Story Listing pages should continue to render correctly with working pagination

[jtmst]

@tjheffner This looks good to me but I cant approve as its originally my PR

[ryguyk]

This looks good overall. I have the one small question about link vs entityPath which I think we can consider, but the much larger question is around how Events is currently working (content-build and vets-website), whether it's correct, and whether we need to/should mimic that or, perhaps, if we should try to get this thing correct while we build it here. That is a much larger issue to consider.

[ryguyk]

I find myself wondering why we have these defined differently. News stories normalize to link whereas events normalize to entityUrl.path.

Should we try to keep that consistent between entity types?

1. If the answer is yes, we could leave this code untouched and adjust other code to settle on one approach.
2. If the answer is no, and there's a good reason to normalize these differently, I wonder why we are setting a link value here rather than keeping it entityUrl.path.

[tjheffner]

I would love this to be consistent across entity types, but I think events as currently constructed would be a larger refactor to do so. I would like to punt on this as I don't think "fixing" events is necessarily within the scope of AP. We should provide the framework for the other product teams to update their products as they are able to do so.

Coordinating events changes between content-build, vets-website, and next-build feels like a larger rabbit hole than we have time for. Ideally, the events changes can happen in vets-website and next-build after content-build is retired.

[ryguyk]

Since we aren't treating event listings the same as story listings with regard to generating subsequent pages, I initially wondered if we actually wanted to consider it a listing resource and add to LISTING_RESOURCE_TYPES. But, there's this code block from src/lib/drupal/staticPaths.ts that reminds me it's important to treat it as a listing resource from the standpoint of removing Lovell federal pages from the build:

  if (isListingResourceType(resourceType)) {
    const lovellFederalRemoved = removeLovellFederalPathResources(resources)
    return await getAllPagedListingStaticPathResources(
      lovellFederalRemoved,
      resourceType as ListingResourceType
    )
  }

[ryguyk]

So there are really two approaches:

1. As is here. Leave it as a "listing resource" but hard-code totalPages to 1.
2. Do not consider it a "listing resource" and then change checks like the one shown in the above comment to instead check another config value like LOVELL_REMOVE_FEDERAL_RESOURCE_TYPES.

For now, I like sticking with option 1, but I'm noting here because it might be the case in the future that additional business logic for "listing resources" could possibly make us have to do additional things for "real" listing resources (i.e. multiple static pages generated in staticPaths) vs. "browser-widget" listing resources (i.e. single page generated and uses a client-side UI to navigate pages) that would make them more different than they are similar. In that case, it would maybe make sense to separate them more completely and handle their similarities as the exception rather than their differences as the exception.

[tjheffner]

I lean towards option 1. Personally, I think it makes more sense conceptually to treat "browser-widget" listing resources as "listing resources" with a hardcoded totalPages of 1.

essentially, listing page without extra generated pages for pagination

[ryguyk]

I'm not sure this is enough. In content-build, both pastEvents and allEventTeasers are populated. However, it's not clear if that is actually working correctly either. See this Slack discussion.

[tjheffner]

I think this is working correctly in that the functionality of the filters matches what prod does. The list appears to filter to the same results whether allEventTeasers is included or not. Open to opinions otherwise, but again, I think refactoring the Events Listing widget from vets-website is out of scope for our team.

[tjheffner]

approving, will follow up in future PRs