USEPA / haztrak

An open-source web app that illustrates how waste management software can interface with RCRAInfo to track hazardous waste electronically
https://usepa.github.io/haztrak/
MIT License
45 stars 19 forks source link

Add debounce to transporter search #727

Closed sheckathorne closed 3 weeks ago

sheckathorne commented 1 month ago

:rocket: Feature Request

Within the manifest form, I think it would be nice from a UX perspective to not require five characters before returning transporter results. I was experimenting with removing the character count requirement and instead adding a debounce filter on the input instead to limit requests to the API.

In the following example, I paused my search twice to refine my search and made two requests to the server - however, I noticed there is a third request with an empty epaId querystring after selecting the transporter.

https://github.com/USEPA/haztrak/assets/21269876/6d0f6146-2e62-40f7-a173-8d02ba92e127

dpgraham4401 commented 1 month ago

This is a great idea, and I agree that it would improve the user experience and has the added benefit of reducing excessive traffic between Haztrak and the e-Manifest system (also I appreciate the video). We do need to consider the constraints of the e-Manifest Site Search service which we call on the back end if the user's org has provided API credentials. The epaSiteId field needs to be at least 2 characters.

I believe we still need to implement that constraint in our back end. I see a couple options:

Option A:

  1. Implement the proposed debounce filter
  2. Remove the 5-char minimum in our front end
  3. Implement the 2-char minimum validation in our back end. If validation fails, just return the sites found in the Haztrak DB (same logic as if the user's org has not supplied e-Manifest API credentials).

Option B:

  1. Implement the proposed debounce filter
  2. Implement the 2 character minimum validation in our back end
  3. Reduce the 5-char minimum to a 2-char minimum in our front end.

I'm partial to the first option A, and seems inline with your initial proposal.

Let me know what you think @sheckathorne.

sheckathorne commented 1 month ago

Honestly, I don't think I understand the implications of A versus B as well as you, so I would lean with you toward option A if you think that's best.

With that said, I think the implementation on the back end would look like this. Let me know if you had something different in mind:

# /apps/rcrasite/services/rcra_site_services.py
class RcraSiteService:

   ...

  def get_or_pull_rcra_site(self, site_id: str) -> RcraSite:
      """
      Retrieves a rcra_site from the database or Pull it from RCRAInfo.
      This may be trying to do too much
      """
      MIN_EPA_ID_LENGTH: int = 2
      if (
          RcraSite.objects.filter(epa_id=site_id).exists()
          and len(site_id) < MIN_EPA_ID_LENGTH
      ):
          logger.debug(f"using existing rcra_site {site_id}")
          return RcraSite.objects.get(epa_id=site_id)
      new_rcra_site = self.pull_rcrainfo_site(site_id=site_id)
      logger.debug(f"pulled new rcra_site {new_rcra_site}")
      return new_rcra_site
dpgraham4401 commented 1 month ago

We went ahead and implemented a check using the view's DRF Serializer class in #728 like so

    class HandlerSearchSerializer(serializers.Serializer):
        siteId = serializers.CharField(required=True)
        siteId = serializers.CharField(
            required=True,
            min_length=2,
        )
        ...

I also went ahead and implemented a new RcraSiteSearch class (maybe overkill) which validates a 2-character minimum before calling the e-Manifest site search service as well.

A future PR for this can also remove the logic in the front end for a 5-character minimum

dpgraham4401 commented 3 weeks ago

closed with #729

Thanks again @sheckathorne. We appreciate your input on ways that we could enhance this project nad hope to see more awesome feature requests like these.