Skip to content

Allow filtering workflows to only running, & add dashboard links #2607

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

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

emma-sg
Copy link
Member

@emma-sg emma-sg commented May 13, 2025

Closes #2527

Changes

  • Adds backend support for filtering workflows by running state
  • Adds "Running" filter alongside "All", "Scheduled", and "No Schedule" options
  • Persists filter state in URL search params, allowing for linking to a specific filter
  • Updates SearchParamsController with a more flexible API
    • Renames set method to update
    • Adds new set and delete methods that mirror the ones on URLSearchParams
  • Adds links to dashboard

Testing

Since this PR involves both frontend and backend changes, you'll need to run the backend locally (or deploy this on dev) to test this out.

  1. Start a crawl
  2. Navigate to the dashboard. Once the crawl has moved past the "starting" phase, verify that the dashboard shows that there's (at least) one running crawl.
  3. Click the "N Crawls Running" text, and verify that it takes you to the crawling page with the "Running" filter selected
  4. Click on various other filters (All, Scheduled, etc) and verify that the list updates to match
  5. Using your browser's back and forward buttons, verify that the filters update when navigating
  6. On an org with more than one page of workflows, verify that you can navigate through multiple pages of workflows and use filters; and that they don't interfere with each other

Screenshots

Workflow List Filter Dashboard Section
Screenshot 2025-05-13 at 6 44 57 PM Screenshot 2025-05-13 at 7 22 33 PM
Filters in URLs
Screenshot 2025-05-13 at 6 47 48 PM
Screenshot 2025-05-13 at 6 48 18 PM
Screenshot 2025-05-13 at 6 48 31 PM
Screenshot 2025-05-13 at 6 47 41 PM

emma-sg added 6 commits May 13, 2025 18:16
- renames `set` method to `update`
- adds `set` and `delete` methods that mirror `URLSearchParams` methods
- add "running" filter
- persist these filters in url search params
- add link to running workflows
- remove dt/dd formatting so that underlines can be rendered
- use tabular numbers in overview
@emma-sg emma-sg requested review from SuaYoo, tw4l and ikreymer May 13, 2025 22:53
@emma-sg emma-sg self-assigned this May 13, 2025
@emma-sg emma-sg added front end Requires front end dev work back end Requires back end dev work labels May 13, 2025
@emma-sg emma-sg changed the title Allow filtering workflows to only running, & add dashboard link Allow filtering workflows to only running, & add dashboard links May 13, 2025
- add more dashboard links as well
Co-authored-by: sua yoo <sua@suayoo.com>
@emma-sg emma-sg requested a review from SuaYoo May 14, 2025 01:05
@click=${() =>
(this.filterBy = {
@click=${() => {
this.searchParams.set("filter", "unscheduled");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since search params are coupled withthis.filterBy, it should probably be reactive, something like (untested:)

updated(changedProperties) {
  if (changedProperties.has("filterBy")) {
    this.searchParams.delete("page");

    if (this.filterby.schedule) {
      this.searchParams.set("filter", "unscheduled");
      // etc, more logic
    }
  }
}

Copy link
Member Author

@emma-sg emma-sg May 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I guess that works, but it feels awkward to me, there's a lot of indirection:

  protected updated(
    changedProperties: PropertyValues<this> & Map<string, unknown>,
  ) {
    if (changedProperties.has("filterBy")) {
      this.searchParams.delete("page");
      if (
        this.filterBy.schedule === undefined &&
        this.filterBy.isCrawlRunning === undefined
      ) {
        this.searchParams.delete("filter");
      } else if (
        this.filterBy.schedule === true &&
        this.filterBy.isCrawlRunning === undefined
      ) {
        this.searchParams.set("filter", "scheduled");
      } else if (
        this.filterBy.schedule === false &&
        this.filterBy.isCrawlRunning === undefined
      ) {
        this.searchParams.set("filter", "unscheduled");
      } else if (
        this.filterBy.schedule === undefined &&
        this.filterBy.isCrawlRunning === true
      ) {
        this.searchParams.set("filter", "running");
      }
    }
  }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this indicates that the search params could be more mappable, something like ?scheduled=true/false, ?running=true/false? Maybe a simplified transformer since setFiltersFromSearchParams feels similarly expanded?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I found a pretty good solution in 3024f6d — with a more expanded search param schema I'd also want more UI that maps to that, and I think the current setup functions more like presets than it does a full set of filter controls. I'm not opposed to the idea in the future, but I think it'd take a bit more UI design first.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, I think I've come around on this, I do see the benefit when it comes to decoupling UI from stored state a bit more. I took a go at this in e83686b, let me know what you think?

@emma-sg emma-sg requested a review from SuaYoo May 14, 2025 17:37
@emma-sg emma-sg marked this pull request as draft May 27, 2025 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
back end Requires back end dev work front end Requires front end dev work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: Display running crawls in dashboard
2 participants