Skip to content

batches: add batch changes badge and filtered page for repo

Warren Gifford requested to merge kr/batch-changes-badge into main

Created by: courier-new

Closes #20309 (closed).

Hello, fellow batchers, and welcome to episode 1 of Repo Batch Changes Badge, otherwise known as RBCB! The synopsis is simple but compelling:

An unsuspecting Sourcegraph user discovers a treasure trove hidden right beside their repositories. What will they do when they uncover... batch changes??

In this pilot episode, we'll introduce you to the full cast of characters and tease the long-running plot, essentially a whole new mode of interacting with existing batch changes from a repository. And we promise there's no cliffhanger ending.

The cast

(aka the main things I'd love for you to pay attention to in this early review 😛)

The Batch Changes Badge

Your charming, enigmatic protagonist who only shows up when there's something exciting happening.

Though I haven't finished hooking this up to GraphQL yet, I was imagining that for visibility we show this badge whenever either

  • The number of open changesets on this repository is > 0, or
  • The number of merged changesets on this repository is > 0,

as both would provide something for the user to look at when they get to the new repo page and draw positive attention to batch changes, even if it's not much.

The Repo Batch Changes Page

They've got lots to show and tell, but you feel like you've seen it all before...

(...that's because you have -- I basically created a mash-up of components from the BatchChangeListPage and the BatchChangeDetailsPage)

The screenwriters couldn't totally agree on the character design and landed somewhere between this and this; they mostly just wanted the character to feel natural amidst the characters it draws inspiration from.

The search + filters, bulk actions, and stats aren't fully wired up to GraphQL yet either because I wanted to get feedback on if they all make sense here or not first. The interesting thing to note about the search and filters is that they'd be applied to the individual changesets, not the parent batch changes, as I felt it made more sense that a user would want to operate on the specific changesets that are attached to this repository from here, and not so much the batch changes they originated from. This will also make wiring these up slightly more complicated, but my plan is to apply them to the nested changesets query and then hide the batch changes from the list that don't have any changesets matching the filters.

I will probably initially release this without bulk action support because the bulk action mutations currently require the batchChange id as an argument in order to check permissions, which gets more complicated if our bulk action is happening across multiple batch changes. I can just hide the checkboxes initially.

I have chosen for now to omit the Open/Closed label on the batch change itself, as well, as it doesn't feel like it adds much to this page. I'm also not yet sold on the visual indicator for the batch change that we had sketched up here on the right:

image

If anyone feels particularly strongly about either of those elements I'm happy to experiment though.

The new Repo query fields

They're the new kid in town who is interested in one thing and one thing only: their repository!

I basically replicated the batchChanges, changesetsStats, and diffStat fields from elsewhere on the schema and tailored them to the details these pages want for a specific repository. The changesetStats will power both the Batch Changes Badge as well as the stats row at the top of the Repo Batch Changes Page.

With the exception of the bulk action support described above, there were no additional mutation requirements needed for this new badge and page.

On the next episode...

(aka questions that I would love your input on!!)

  1. How do repo permissions work? Is it possible for a user to view this (or any other) repo page on Sourcegraph for a repo they don't have access to on the codehost? Would they just see a bunch of HiddenExternalChangesets if they did? A: It shouldn't be possible.
  2. Does having search + filters for changesets (vs. for the batch changes themselves) make sense on this list? A: Exclude search + filters from initial release.
  3. Which changesets stats should we include on the row at the top of the page? Currently I have total, open, unpublished, closed, and merged. A: Seems like a good set. 3a. If a given stat has 0 of its type (e.g. 0 closed changesets), should I omit the stat from the row, or keep it and just show 0? A: Keep it and show 0.
  4. Erik already gave me some pointers on handling the enterprise-in-OSS parts of this, is there anything else I missed here? A: Looks good.
  5. I am generally erring on the side of duplicating code here over generalizing existing code or forming new abstractions when it would only serve two places. I'm going to do a lap of cleanup and assess if any of these places make sense to DRY up, but if anything jumps out at you as being better served that way, please suggest it!
  6. What stories should we include for the Repo Batch Changes page? I currently have with a full list, with a list with hidden external changesets, and with no batch changes. A: Can safely exclude the middle one, shouldn't be possible. No others needed.

Plot holes

(aka tracking the things that I'm still working on/cleaning up for this PR)

  • Wire up changeset stats queries on the frontend
  • Wire up diff stat query on the frontend
  • Wire up changeset search + filters query parameters
  • Hide a batch change from the list if all of its changesets are hidden by the current search parameters
  • Clean up repo permissions
  • Exclude archived changesets from queries, stats
  • Write backend tests for new GraphQL queries
  • If page is empty, point user to batch changes getting started materials
  • Fix initial page title
  • File issue to track enhancements: bulk action support, search + filters
  • CHANGELOG entry

Merge request reports

Loading