Skip to content
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

fixes #65 issue when expanding list of multiple relations #66

Merged
merged 1 commit into from
Aug 31, 2023
Merged

fixes #65 issue when expanding list of multiple relations #66

merged 1 commit into from
Aug 31, 2023

Conversation

m29h
Copy link
Collaborator

@m29h m29h commented Aug 9, 2023

This adds both a integration test case for multiple relation expansion as well as resolves #65

we did not handle the case where the expansion is a list of records instead of a single record directly

Note: this only pushes the uncertainty down to the next layer.
Now the value of expand["xxx"] can either be

  • not present in the map expand at all if there is no relation
  • a value of type Record if there actually is a single relation (even if pocketbase theoretically allows to set more than one relation)
  • a value of type List[Record] if there are actually two or more relations

@coveralls
Copy link

coveralls commented Aug 9, 2023 •
edited
Loading

Pull Request Test Coverage Report for Build 5814928105

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 92.042%
Totals Coverage Status
Change from base Build 5290254865: 0.1%
Covered Lines: 801
Relevant Lines: 843
💛 - Coveralls

@m29h m29h mentioned this pull request Aug 9, 2023
Closed
@m29h m29h requested a review from vaphes August 10, 2023 16:22
@kbruegge
Copy link

So this seems to work. It now returns a list of records. Which for me is absolutely fine.

In [7]: r.expand
Out[7]: {'location': [<Record: 0yoeep2hyh0p57t>]}

Thanks!

@Cobryis
Copy link

Cobryis commented Aug 26, 2023

@vaphes Are you planning on integrating this fix?

@vaphes vaphes merged commit a840a1b into vaphes:master Aug 31, 2023
@m29h m29h deleted the fix_issue_65 branch September 15, 2023 20:44
@m29h m29h linked an issue Dec 13, 2023 that may be closed by this pull request
Back-relation expand #73
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vaphes vaphes vaphes approved these changes

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

Successfully merging this pull request may close these issues.

Back-relation expand TypeError when expanding relations.
5 participants
@m29h @coveralls @kbruegge @Cobryis @vaphes