Skip to content

Conversation

@mfonism
Copy link
Contributor

@mfonism mfonism commented Sep 16, 2021

The API method TestCase.filter is extended to work for queries that contain lookups on the fields setup_duration, testing_duration and expected_duration.
It also returns the values of all three fields in its result.

Refs #1923

The API method `TestCase.filter` is extended to work for queries that
contain lookups on the fields `setup_duration`, `testing_duration` and
`expected_duration`.
It also returns the values of all three fields in its result.

Refs kiwitcms#1923
default=F("setup_duration") + F("testing_duration"),
)
)
.filter(**query)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like there may be some confusion around this item. Being able to filter by the duration field is the last checklist item in #1923. That should not need special casing for setup_duration and testing_duration because these are model fields. The only special case is expected_duration.

OTOH the item "API method TestCase.filter returns the value of all 3 fields - tests are updated to match" refers only to making sure that the API method TestCase.filter includes the new fields in its results. That should be much easier and require a lot less changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh oh. I was over thinking it.

I thought the current checklist item also meant that filtering with respect to the duration fields should be possible on the API side.

I will create a new PR to implement just the presence of the duration fields in the result.

I'll keep this one alive (closed) so one can refer to it if we ever need to add filtering with respect to the duration fields on the API side.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It just occured to me that I can shorten the entire Case expression with Coalesce:

Coalesce("setup_duration", timedelta(0)) + Coalesce("testing_duration", timedelta(0))

if duration is None:
continue

query[key] = duration
Copy link
Contributor Author

@mfonism mfonism Sep 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this for loop because the values of the duration keys in the incoming dict query will always be strings as xmlrpc can't marshal them if they are datetime.duration objects. The loop parses these strings (if they are present) into datetime.duration objects -- so that QuerySet.filter() won't choke on them.

Copy link
Contributor Author

@mfonism mfonism Sep 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we don't need to filter by duration fields on the API side, then I'll take this out.

Although if someone ever mistakenly sends in a lookup based on a duration field, it will choke.

)
for testcase_dict in qs.iterator()
]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I loop through the list of dicts (representing testcases) and make sure that if the value of the duration keys are not null, they are converted from duration objects to strings as xmlrpc cannot marshal duration objects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants