Feature #20470
closedSelect fields with group contents query
100%
Related issues
Updated by Peter Amstutz about 1 month ago
- Status changed from New to In Progress
Updated by Peter Amstutz about 1 month ago
- Related to Bug #20469: Responsiveness of viewing project full of running processes added
Updated by Peter Amstutz about 1 month ago
20470-contents-select @ f4e3341d2857114e8c3ea0a7941ff8429491dd59
- Make "select" parameter work for group contents
- Each column is either prefixed (selects a specific column of a specific table) or unprefixed (selects that column for all tables, e.g. "uuid")
Updated by Peter Amstutz about 1 month ago
20470-contents-select @ 385d9ac5f47bec5c5a5fc9770c74b1a7d8dd2974
- Fix nil case
Updated by Peter Amstutz 29 days ago
20470-contents-select @ ff843a40de882d22dd2a5e408c77a2fa4720cc7d
- fix tests
Updated by Peter Amstutz 29 days ago
Failing test:
14:45:19 FAIL: collection_test.go:249: CollectionSuite.TestSignatures 14:45:19 14:45:19 collection_test.go:318: 14:45:19 c.Check(gresp.Items[0].(map[string]interface{})["manifest_text"], check.Equals, nil) 14:45:19 ... obtained string = ". acbd18db4cc2f85cedef654fccc4a4d8+3 0:3:foo\n" 14:45:19 ... expected = nil
What is happening here is that the test Select: []string{"uuid", "manifest_text"}
on the GroupContents request. However this was a no-op before. So the test is buggy. What changed is that select now does something and manifest_text
is returned if you ask for it.
I think maybe the fix here is to split the test into one version that doesn't have "select" (so it won't return manifest_text) and the one that does select on "manifest_text" and checks the signature.
Updated by Peter Amstutz 29 days ago
20470-contents-select @ e0f045ddedd05716bd813f801654308d3bbd2dd0
I decided to make the behavior match the test.
Updated by Tom Clegg 28 days ago
It looks like we're now silently ignoring bogus attributes in "select", except in the specific case where all select attributes are bogus, where we now return a somewhat misleading/confusing error message. This doesn't seem ideal.
I think we could maintain the error-checking behavior by doing "customize @select
for each table" in GroupsController.load_searchable_objects, similar to limit_all/offset_all. We could explicitly check at the top that each "select" entry is valid for at least one klass, and error out if not. Then set @select
to the appropriate subset for each klass inside the loop.
Updated by Peter Amstutz 28 days ago
20470-contents-select @ 2c854a99434c76e0165543bedfe9e97a1fe81ae8
- make the error reporting which 'select' field is invalid work again
wb1 re-run: developer-run-tests-apps-workbench-integration: #3928
Updated by Tom Clegg 28 days ago
Could we have a comment in ContainersController#find_objects_for_index and ApplicationController#find_object_by_uuid, about why @preserve_select
is needed? I think the reason is to ensure ApplicationController#object_list uses the same set of fields.
@select = @preserve_select = %w(uuid state priority auth_uuid locked_by_uuid)
I'm a little confused by this code -- it looks like it will incorrectly accept "collections.bogus" without checking it, but I couldn't make it behave badly in a test case.
sp = column.split(".")
if sp.length == 2 && sp[0] == model_class.table_name
sp[1]
elsif model_class.selectable_attributes.include? column
column
elsif raise_unknown
raise ArgumentError.new("Invalid attribute '#{column}' of #{model_class.name} in select parameter")
else
nil
end
Updated by Peter Amstutz 28 days ago
20470-contents-select @ 9d3ace1fdba783134eb5557a9b28f8132df552de
- Use selectable_attributes to determine which fields are published in the schema
- Don't accept "collections.bogus" as a selectable field
- Update comments to note the @preserve_select workaround
wb1 re-run developer-run-tests-apps-workbench-integration: #3934
Updated by Peter Amstutz 27 days ago
- Status changed from In Progress to Resolved
Updated by Brett Smith 14 days ago
- Precedes Bug #20527: Document new select parameter of groups.contents API added