Project

Profile

Help

Issue #3558

closed

Repository version filters are confusing

Added by daviddavis over 6 years ago. Updated almost 5 years ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
Normal
Assignee:
Category:
-
Sprint/Milestone:
Start date:
Due date:
Estimated time:
Severity:
2. Medium
Version:
Platform Release:
OS:
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
Documentation
Sprint:
Sprint 35
Quarter:

Description

I was working with kersom who was having trouble with the repo version filters.

First, there's a content filter of type 'string' but there's no description or indication of what it expects. Recommend adding a description. Also, is there a type we can use like 'href' or something?

Then there's version_min and version_max. First off, there's no version field on repo version. It's actually called 'number'. Secondly, there's no indication in the api schema whether version_min=1 will include repo version 1 or not. We could maybe just use 'gt', 'gte', 'lt', 'lte' like django-filters.

edit: The we should remove the "version_min" and "version_max" filters and expose the number field to filter on directly, e.g. "number__lte"

Actions #1

Updated by daviddavis over 6 years ago

  • Description updated (diff)
  • Tags Documentation added
Actions #2

Updated by amacdona@redhat.com over 6 years ago

  • Triaged changed from No to Yes

Since "version" is not really a field, we should not include that. "number" works, but may be unnecessary. I think we should use max and min (+1)

versions/?min=1, max=4

That looks pretty clear to me. If we do gte/lte, it requires us to name the field. (+0)

versions/?number__gte=1,number__lte=4

Actions #3

Updated by dalley over 6 years ago

I would actually prefer to rename the field for reasons laid out here:

https://pulp.plan.io/issues/3557#note-2

I'm not entirely comfortable with "max" and "min" because there are enough fields on a RepositoryVersion that it isn't immediately clear which one they would correspond to. And "number" doesn't even have any help text at the moment.

I'm also not against it, it is nice and short (+0 I guess), I would just favor consistency over minor convenience.

Actions #4

Updated by amacdona@redhat.com over 6 years ago

I like the user experience of renaming "number" to "version" for this filter, but it just seems weird elsewhere.

To me, "RepositoryVersion.version" is awkward "RepositoryVersion.number" makes sense. Rename to "version" -0

I'm convinced that we can use "*__gte" for the field. As it is, "number__gte" seems fine. +1

Actions #5

Updated by dalley over 6 years ago

To clarify - I did mean "filter param" instead of field. I'm +1 on just having "number__gte" and "number__lte" instead of version_min and version_max.

Actions #6

Updated by dalley over 6 years ago

  • Description updated (diff)
Actions #7

Updated by dalley over 6 years ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to dalley
Actions #8

Updated by dalley over 6 years ago

  • Status changed from ASSIGNED to POST
  • Sprint set to Sprint 35
Actions #9

Updated by bmbouter over 6 years ago

+1 on this resolution.

Added by dalley over 6 years ago

Revision 56a553a3 | View on GitHub

Fix confusing inconsistencies with RepositoryVersion filters

closes #3558 https://pulp.plan.io/issues/3558

Added by dalley over 6 years ago

Revision 56a553a3 | View on GitHub

Fix confusing inconsistencies with RepositoryVersion filters

closes #3558 https://pulp.plan.io/issues/3558

Actions #10

Updated by dalley over 6 years ago

  • Status changed from POST to MODIFIED
Actions #11

Updated by dkliban@redhat.com over 6 years ago

  • Sprint/Milestone set to 3.0.0
Actions #12

Updated by bmbouter over 5 years ago

  • Tags deleted (Pulp 3, Pulp 3 MVP)
Actions #13

Updated by bmbouter almost 5 years ago

  • Status changed from MODIFIED to CLOSED - CURRENTRELEASE

Also available in: Atom PDF