Skip to content

fix: Query conditions inQuery and notInQuery not working properly #869

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
May 17, 2023
Merged

fix: Query conditions inQuery and notInQuery not working properly #869

merged 10 commits into from
May 17, 2023

Conversation

mbfakourii
Copy link
Member

New Pull Request Checklist

Issue Description

There was an unused "," value in the _buildQueryRelational function

Closes: #849

Approach

n/a

TODOs before merging

  • Add tests
  • Add changes to documentation (guides, repository pages, in-code descriptions)
  • A changelog entry

@parse-github-assistant
Copy link

parse-github-assistant bot commented May 5, 2023 •
edited
Loading

Thanks for opening this pull request!

  • 🎉 We are excited about your hands-on contribution!

@mbfakourii mbfakourii changed the title fix:_buildQueryRelational function unused "," value in inQuery and notInQuery function fix: _buildQueryRelational function unused "," value in inQuery and notInQuery function May 5, 2023
@codecov
Copy link

codecov bot commented May 5, 2023 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@d1b07fe). Click here to learn what that means.
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 934a090 differs from pull request most recent head 98a24cd. Consider uploading reports for the commit 98a24cd to get more accurate results

Additional details and impacted files 
@@            Coverage Diff            @@
##             master     #869   +/-   ##
=========================================
  Coverage          ?   39.13%           
=========================================
  Files             ?       56           
  Lines             ?     3278           
  Branches          ?        0           
=========================================
  Hits              ?     1283           
  Misses            ?     1995           
  Partials          ?        0
Impacted Files Coverage Δ
packages/dart/lib/src/network/parse_query.dart 32.42% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@mtrezza
Copy link
Member

mtrezza commented May 7, 2023

Is this ready for review?

@mbfakourii mbfakourii requested a review from a team May 7, 2023 17:50
@mbfakourii
Copy link
Member Author

Is this ready for review?

Yes, it is ready for review

Nidal-Bakir
Nidal-Bakir previously approved these changes May 7, 2023
View reviewed changes
@Nidal-Bakir
Copy link
Member

Can you add a test to ensure that this does not happen again?

@mbfakourii
Copy link
Member Author

Can you add a test to ensure that this does not happen again?

Yes, sure

@mbfakourii mbfakourii requested a review from a team May 12, 2023 08:34
@mbfakourii
Copy link
Member Author

@mtrezza
Why did half of the CI failed?

@Nidal-Bakir
Copy link
Member

Nidal-Bakir commented May 12, 2023 •
edited
Loading

Why did half of the CI failed?

See #874 and #872 (comment)

@mbfakourii
Merge branch 'master' into fix_bug_InQuery_and_notInQuery
9705396 
# Conflicts:
#	packages/dart/test/src/network/parse_query_test.dart
Nidal-Bakir
Nidal-Bakir suggested changes May 15, 2023
View reviewed changes
Nidal-Bakir
Nidal-Bakir previously approved these changes May 15, 2023
View reviewed changes
Copy link
Member

@Nidal-Bakir Nidal-Bakir left a comment

Choose a reason for hiding this comment

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

Thanks

mtrezza
mtrezza approved these changes May 17, 2023
View reviewed changes
Copy link
Member

@mtrezza mtrezza left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good, could you just rephrase the PR title, it seems to describe the technical solution instead of the bug that is fixed by this PR; so that we have a meaningful changelog entry

@mbfakourii mbfakourii changed the title fix: _buildQueryRelational function unused "," value in inQuery and notInQuery function fix: inQuery and notInQuery function in Parse query May 17, 2023
@mbfakourii
Copy link
Member Author

Looks good, could you just rephrase the PR title, it seems to describe the technical solution instead of the bug that is fixed by this PR; so that we have a meaningful changelog entry

I changed
The problem is very small. I don't have a better idea for the title

@mbfakourii mbfakourii changed the title fix: inQuery and notInQuery function in Parse query fix: inQuery and notInQuery function bug in Parse query May 17, 2023
@mtrezza mtrezza changed the title fix: inQuery and notInQuery function bug in Parse query fix: Query conditions inQuery and notInQuery now working properly May 17, 2023
@mtrezza mtrezza changed the title fix: Query conditions inQuery and notInQuery now working properly fix: Query conditions inQuery and notInQuery not working properly May 17, 2023
mtrezza
mtrezza requested changes May 17, 2023
View reviewed changes
Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

Could you add a changelog entry?

@mbfakourii
Copy link
Member Author

Could you add a changelog entry?

added

mtrezza
mtrezza approved these changes May 17, 2023
View reviewed changes
Copy link
Member

@mtrezza mtrezza left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good! Waiting for CI...

@mtrezza mtrezza merged commit 8d6566f into parse-community:master May 17, 2023
@mbfakourii mbfakourii deleted the fix_bug_InQuery_and_notInQuery branch May 23, 2023 11:40
@Nidal-Bakir Nidal-Bakir mentioned this pull request Jun 5, 2023
Open
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mtrezza mtrezza mtrezza approved these changes

@Nidal-Bakir Nidal-Bakir Nidal-Bakir left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Including pointer in another pointer not working
3 participants
@mbfakourii @mtrezza @Nidal-Bakir