Project

Profile

Help

Issue #3558

Repository version filters are confusing

Added by daviddavis over 1 year ago. Updated 8 months ago.

Status:
MODIFIED
Priority:
Normal
Assignee:
Category:
-
Sprint/Milestone:
Start date:
Due date:
Severity:
2. Medium
Version:
Platform Release:
Blocks Release:
OS:
Backwards Incompatible:
No
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
Documentation
QA Contact:
Complexity:
Smash Test:
Verified:
No
Verification Required:
No
Sprint:
Sprint 35

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"

Associated revisions

Revision 56a553a3 View on GitHub
Added by dalley over 1 year ago

Fix confusing inconsistencies with RepositoryVersion filters

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

Revision 56a553a3 View on GitHub
Added by dalley over 1 year ago

Fix confusing inconsistencies with RepositoryVersion filters

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

History

#1 Updated by daviddavis over 1 year ago

  • Description updated (diff)
  • Tags Documentation added

#2 Updated by amacdona@redhat.com over 1 year 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

#3 Updated by dalley over 1 year 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.

#4 Updated by amacdona@redhat.com over 1 year 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

#5 Updated by dalley over 1 year 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.

#6 Updated by dalley over 1 year ago

  • Description updated (diff)

#7 Updated by dalley over 1 year ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to dalley

#8 Updated by dalley over 1 year ago

  • Status changed from ASSIGNED to POST
  • Sprint set to Sprint 35

#9 Updated by bmbouter over 1 year ago

+1 on this resolution.

#10 Updated by dalley over 1 year ago

  • Status changed from POST to MODIFIED

#11 Updated by dkliban@redhat.com over 1 year ago

  • Sprint/Milestone set to 3.0

#12 Updated by bmbouter 8 months ago

  • Tags deleted (Pulp 3, Pulp 3 MVP)

Please register to edit this issue

Also available in: Atom PDF