Skip to content

Change REST API's __contains to OR behavior#3531

Merged
tardyp merged 2 commits intobuildbot:masterfrom
jangmarker:change-rest-contains-to-or-behavior
Aug 18, 2017
Merged

Change REST API's __contains to OR behavior#3531
tardyp merged 2 commits intobuildbot:masterfrom
jangmarker:change-rest-contains-to-or-behavior

Conversation

@jangmarker
Copy link
Copy Markdown
Contributor

Created out of a discussion in #3526.
Fixes #3527.

Contributor Checklist:

  • I have updated the unit tests
  • I have created a file in the master/buildbot/newsfragment directory (and read the README.txt in that directory)
  • I have updated the appropriate documentation

@mention-bot
Copy link
Copy Markdown

@jangmarker, thanks for your PR! By analyzing the history of the files in this pull request, we identified @rodrigc, @tardyp and @delanne to be potential reviewers.

1 similar comment
@mention-bot
Copy link
Copy Markdown

@jangmarker, thanks for your PR! By analyzing the history of the files in this pull request, we identified @rodrigc, @tardyp and @delanne to be potential reviewers.

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 16, 2017

Codecov Report

Merging #3531 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3531   +/-   ##
=======================================
  Coverage   88.27%   88.27%           
=======================================
  Files         323      323           
  Lines       33780    33780           
=======================================
  Hits        29818    29818           
  Misses       3962     3962
Impacted Files Coverage Δ
master/buildbot/data/resultspec.py 90.98% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d12e5f...20884ae. Read the comment docs.

Although the documentation suggests that multiple __contains filter
where connected with OR (similar to __eq and __ne) they were connected
with AND. Fix this behavior.
@jangmarker jangmarker force-pushed the change-rest-contains-to-or-behavior branch from ca9eb82 to c0754c3 Compare August 16, 2017 08:41
select resources where the field's value is greater than or equal to ``{value}``
``contains``
select resources where the field's value contains ``{value}``
select resources where the field's value contains ``{value}``, or with the same parameter appearing multiple times contains one of the values (so `foo__contains=x&foo__contains=y` would match resources where foo contains `x`, `y` or both)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is an awkwardly long sentence. The addition in this PR should be its own sentence.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've split the sentences.

@jangmarker jangmarker force-pushed the change-rest-contains-to-or-behavior branch from c0754c3 to 20884ae Compare August 17, 2017 02:02
@jangmarker
Copy link
Copy Markdown
Contributor Author

@seankelly @tardyp thanks for your reviews! Can this be merged then? I would like to rebase #3526 after this has been merged.

@tardyp tardyp merged commit 6a96902 into buildbot:master Aug 18, 2017
@jangmarker jangmarker deleted the change-rest-contains-to-or-behavior branch September 7, 2017 09:38
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.

4 participants