-
Notifications
You must be signed in to change notification settings - Fork 9
Enhancement/allow adding removing of queues of user #83
Enhancement/allow adding removing of queues of user #83
Conversation
Codecov Report
@@ Coverage Diff @@
## master #83 +/- ##
=========================================
+ Coverage 91.24% 91.64% +0.4%
=========================================
Files 24 25 +1
Lines 1336 1401 +65
=========================================
+ Hits 1219 1284 +65
Misses 117 117
Continue to review full report at Codecov.
|
0b90d0f
to
66938d2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have no better suggestion for the command name. I think the future tab-completion could solve the problem with the command name length.
@option.user(required=False, help="User IDs, which the queues will be filtered by.") | ||
@option.queue(required=False, help="Queue IDs, which the users will be filtered by.") | ||
@click.pass_context | ||
def list_command(ctx: click.Context, user_ids: Tuple[int], queue_ids: Tuple[int]) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I call user_assignment list
, I intuitively expect to see a table that shows me what user is assigned to what queue, as is the case for all other list subcommands. Instead, I am getting KeyError: 'id'
and need to write help
for more information to find out the ID is required.
Consider making the ID not required and listing a table similar to the table shown after typing user list
. I think it copies better the way the rest of elisctl is used, and the user will probably not know the IDs directly, but will have to find them out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is bug, as it is supposed to work the proposed way (see the example produced in tests below).
elisctl user_assignment list
id username queue id queue name
---- ---------- ---------- ------------
4 user_4 1 TestQueue
2 TestQueue
5 user_5 1 TestQueue
2 TestQueue
Both the ids are non-required.
And I tried it myself for admin role and annotator as well, however, I cannot reproduce the error.
Please provide whole traceback and the user under which you tried this command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(elisctl-forked) C:\PyCharm\Projects\elisctl-forked>pip install -e .
(elisctl-forked) C:\PyCharm\Projects\elisctl-forked>elisctl --version
elisctl, version 2.6.0
(elisctl-forked) C:\PyCharm\Projects\elisctl-forked>elisctl
Welcome to the elisctl interactive mode. Start with `help` and `configure`.
elis> help
Documented commands (type help <topic>):
========================================
configure csv queue tools user_assignment
connector document schema user workspace
Undocumented commands:
======================
exit help quit
elis> user_assignment list
KeyError: 'id'
elis>
Tried with user 13564. queue list
and other commands work ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, the problem is that that queue 18994 has a user from a different organization. I'll take that into account.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, fixed. Please take a look again :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
A subcommand
user_assignment
on the top level was created, which modifies the queues:users relationship. It allows for assignments/removals of:elisctl user_assignment <action> -q 1 -u 1 -u 2
),elisctl user_assignment <action> -q 1 -q 2 -u 1
),elisctl user_assignment <action> -q 1 -u 1
), as well aselisctl user_assignment <action> -q 1 -q 2 -u 1 -u 2 -u 3
).If the user does (in case of assignment) have the queue, nothing happens.
If the user does not (in case of removal) has not the queue, nothing happens.
Also a
elisctl user_assignment list
command was added, which lists assignments of queues to users. Filters can be applied.Closes: #82, #77