Open andrepapoti opened 6 months ago
Sorry for the absurd delay getting to this: my free time for Patchwork is pretty much zero recently... :sweat_smile:
I've taken a look at this locally but have yet to go into depth on it. There are two obvious issues starting out though. Firstly, you need to do some serious work on the queries: I loaded up the patchwork archives (following the guide in our development docs and loading the series-list
page took over 7 seconds and ~3050 queries (:smile:). Secondly, the current iteration of the UI doesn't work and it scrolls past the edge of the screen. I would suggest the following changes:
Series Name
should be the first column and called Series
, as we do for patch list and Patch
Number of Patches
to Patches
Other changes:
/series/{series}
should be /project/{project}/series/{series}
to map with patches/project/patchwork/series-list/
should be /project/patchwork/series/
, and we should rename /project/patchwork/list/
to /project/patchwork/patches/
(with an alias in place).This is rather beefy PR so if you have the ability to do, I'd suggest submitting v2 via the mailing list if you have the ability/know-how to do so. That'll be easier for me to review. Feel free to stick to GitHub if you can't/aren't comfortable using the mailing list though :slightly_smiling_face:
@stephenfin I've completed the adjustments you specified and also added pagination to the series list, the page is displaying 100 series at once and It's making 7 requests per page. I've also made the adjustments for the table display and endpoint urls. I can send this patch via email as well but it may be interesting to keep the discussion on github since the PR is tracking issues that are registered on this repository.
@andrepapoti I gave this a run. Main issue I have noticed is the default sorting, patches in Series view should be sorted from newest to the oldest, like they are in patches. It doesnt really make sense to have it sorted the other way around.
Should a navigation bar within a patch - next/previous be visible If there is a single patch in a series,?
edit: Finally on the sorting bit, patches view allows for sorting by different tables (submitter, state, date etc.) and it allows to reverse sorting as well, so that if something is sorted by date (oldest -> newest), if one clicks date again it changes to newest->oldest, or if one clicks on patch, then it sorts it alphabetically by patch name. I think it would be nice to have that in series view as well, to keep it constant.
Should a navigation bar within a patch - next/previous be visible If there is a single patch in a series,?
There is no planned design for the task, I kept each button disabled if there are no next/previous values, but if you believe it's best to remove them I don't have any issues with it
Regarding the sorting I'll add a new commit to this MR to allow the user to change it by clicking on the table
Just to make this clear. As it is the series view aren't exactly appealing, they show every patch series that has been ever submitted. Series should have states - open/closed, so they can be filtered on the web interface. As mentioned before, series should be sorted by date descending by default, and possibly only series that have state == open should be displayed by default. Because this sounds like a design decision it possibly should be discussed/clarified on mailing list if two states (is_open = True/False) is ok, or if there is a need for lets say new/open/closed/archived set of states like there is for patch. (Personally I think two are fine, but it's no harm asking the community what they think before making a decision).
Description
This PR adds a new series-list and series-detail view. Some projects can have hundreds of patches actives at once, by giving the user the ability to have an overview of all of the series makes knowing what's the current state of a project much more simple. Series-list also allows the user to apply changes to all of the patches of a determined series, making the management of them easier Added navigation within a series for the patch-detail view
Related