Bug 1881668 - hammer user list --help has invalid --order example
Summary: hammer user list --help has invalid --order example
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Satellite
Classification: Red Hat
Component: API
Version: 6.7.0
Hardware: All
OS: Linux
low
low
Target Milestone: 6.11.0
Assignee: aabramov
QA Contact: Lukáš Hellebrandt
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2020-09-22 20:39 UTC by Pavel Moravec
Modified: 2022-11-17 11:43 UTC (History)
10 users (show)

Fixed In Version: foreman-3.1.2,foreman-3.1.1.11-1
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2022-07-05 14:28:25 UTC
Target Upstream Version:
Embargoed:
aabramov: needinfo-


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Foreman Issue Tracker 32703 0 Low Closed hammer user list --help has invalid --order example 2022-04-06 13:54:39 UTC
Foreman Issue Tracker 34381 0 Normal Closed Change example for --order 2022-04-06 13:54:41 UTC
Red Hat Product Errata RHSA-2022:5498 0 None None None 2022-07-05 14:28:37 UTC

Description Pavel Moravec 2020-09-22 20:39:45 UTC
Description of problem:
hammer user list --help shows:

..
 --order ORDER                             Sort field and order, eg. ‘id DESC’
..

There are two errors in the example:

1) quote marks cant be used in a real example:

hammer> user list --order ‘id DESC’
Error: Too many arguments.

See: ' user list --help'.
hammer> 


2) Even with proper quote marks, it does not work:

hammer> user list --order "id DESC"
400 Bad Request
  the field 'id' in the order statement is not valid field for search
hammer> 

(since also WebUI disallows searching by ID of user).


Version-Release number of selected component (if applicable):
Satellite 6.7.2
tfm-rubygem-hammer_cli-0.19.2.1-1.el7sat.noarch


How reproducible:
100%


Steps to Reproduce:
1. hammer user list --help


Actual results:
..
 --order ORDER                             Sort field and order, eg. ‘id DESC’
..


Expected results:
..
 --order ORDER                             Sort field and order, eg. "login DESC"
..


Additional info:

Comment 1 Shira Maximov 2020-10-05 11:17:09 UTC
The BZ Fix should be in the API.

Comment 3 Tomer Brisker 2021-06-03 09:07:06 UTC
Created redmine issue https://projects.theforeman.org/issues/32703 from this bug

Comment 4 Bryan Kearney 2021-06-10 16:05:29 UTC
Moving this bug to POST for triage into Satellite since the upstream issue https://projects.theforeman.org/issues/32703 has been resolved.

Comment 5 Lukáš Hellebrandt 2022-02-01 14:38:35 UTC
FailedQE with Sat 7.0 snap 6.0.

The quotation marks are now correct but the example still uses 'id' column which is still not searchable:

```
# hammer user list --order 'id DESC'
400 Bad Request
  the field 'id' in the order statement is not valid field for search
```

Comment 6 Oleh Fedorenko 2022-02-01 15:13:13 UTC
I'd say it's OK to verify since the description says 

> Sort and order by a searchable field, e.g. 'id DESC'

Please, notice that there is explicit mention of a "searchable" field. The list of searchable fields is different for different entities. So, users entity can lack of this particular (id) searchable field, but other entities have this. This is simply a general description, it's not a call for an action.

The actual support for ordering by id for users was resolved in upstream in https://projects.theforeman.org/issues/33842.

Comment 7 Bryan Kearney 2022-02-01 16:02:44 UTC
Upstream bug assigned to aabramov

Comment 8 Lukáš Hellebrandt 2022-02-01 16:57:47 UTC
Oleh, it's not ok, this description is for `hammer user list` where `id` is never searchable, the example is wrong. (And yes, the general description is correct without the example.)

Comment 9 Oleh Fedorenko 2022-02-02 13:42:31 UTC
The description comes from the API and it's a general description for all the API endpoints and thus hammer commands.

The only way to resolve this BZ is to simply remove the example. Because I don't think it's a good idea to remove something general and add a specific description with a specific example for each endpoint to match the fields.

Unless you can offer a better solution, I'd vote for removal of the example.

Comment 10 Lukáš Hellebrandt 2022-02-02 14:07:18 UTC
How about '<column> DESC' as an example?
It would be more general but less of an example. Still better than removing it completely.
Without the example, it would be totally incomprehensible since there is no description of a correct syntax ('<column> <order>') - which can also be fixed easily.

Comment 12 aabramov 2022-02-03 07:38:46 UTC
Connecting redmine issue https://projects.theforeman.org/issues/34381 from this bug

Comment 13 Lukáš Hellebrandt 2022-04-06 13:33:36 UTC
FailedQA with Sat 6.11 snap 15.0.

# hammer user list --help | grep order
 --order VALUE                           Sort and order by a searchable field, e.g. 'id DESC'

# rpm -qa | grep ^foreman-3
foreman-3.1.1.9-1.el7sat.noarch

Comment 16 Lukáš Hellebrandt 2022-04-14 09:23:02 UTC
Verified with Sat 6.11 snap 16.0.

# rpm -qa | grep ^foreman-3
foreman-3.1.1.12-1.el7sat.noarch

# hammer user list --help | grep order
 --order VALUE                           Sort and order by a searchable field, e.g. '<field> DESC'

# hammer user list --order 'login DESC'
---|-------|------------|------------------------------|-------|----------|---------------------|--------------
ID | LOGIN | NAME       | EMAIL                        | ADMIN | DISABLED | LAST LOGIN          | AUTHORIZED BY
---|-------|------------|------------------------------|-------|----------|---------------------|--------------
4  | admin | Admin User | root@<FQDN> | yes   | no       | 2022/04/14 09:19:29 | Internal     
---|-------|------------|------------------------------|-------|----------|---------------------|--------------

# hammer host list --help | grep order
 --order VALUE                    Sort and order by a searchable field, e.g. '<field> DESC'

=> The help text is now general enough and the quotes are correct. Overall, the help text seems correct and only slightly less explanatory.

Comment 19 errata-xmlrpc 2022-07-05 14:28:25 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Moderate: Satellite 6.11 Release), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2022:5498


Note You need to log in before you can comment on or make changes to this bug.