Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/duplicate board query #61
base: master
Are you sure you want to change the base?
Feature/duplicate board query #61
Changes from 2 commits
e9c2d80
748f418
5480228
5dc386d
ed8e29d
873dda3
f9e3a6e
d707140
12b6834
f196586
89a5b8c
cdeaa30
050c32d
02820ea
0d41eb5
0a9b87d
1a30751
4340a28
cd885f8
1837e1d
ba4e159
af5fa68
bd9358d
ee25c90
6d5b19b
53faa21
2153140
4e2d9a0
85154c8
a528b00
fb9adc2
2b0689e
1d38403
93938a2
9cc8e82
17efb51
2335617
3b2c1f0
0b94fb1
d42b3ec
8f445d5
11ac7b5
5120dbc
5027df3
0d9cf2f
246bfc7
5226ae0
d32dd59
f5cf788
77c9ca3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This looks like it should be indented once more.
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.
Do you mean to move it under the
workspace_id
condition? That condition is meant to only add to the params variable theworkspace_id
parameter for the query but it's not a mandatory condition for the query.However, there is a clear issue there because there isn't a value passed for the
%s
token, and the test didn't catch it because it was expecting to find a 1 and a 2 within the query, but both are included in theboard_id
value (Its value is 12) That was a really bad test.I have updated both the query and its test to fix these issues. I wonder if it had something to do with the merge when the branch was like +20 commits behind...
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.
Honestly, now that I look at it again I'm not sure what I was talking about, it looks fine... disregard.
Good catch on test, and ty for fixing.