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.

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

Don't load all relationship results #5465

Merged
merged 3 commits into from Oct 18, 2021
Merged

Don't load all relationship results #5465

merged 3 commits into from Oct 18, 2021

Conversation

emptynick
Copy link
Collaborator

In a belongs to many relationship all related items were loaded here:
https://github.com/the-control-group/voyager/blob/eb83ecb0f10192eed83dd8d2a3975dc3c8b32252/resources/views/formfields/relationship.blade.php#L174

Although we use ajax to load related items.
Another problem was that enabling tagging disabled ajax which does not make sense at all.

Possibilities tested:

  • Adding
  • Editing
  • Editing with failing validation (selected relationships stay)
  • Tagging
  • No tagging (yes, this needed to be considered)

emptynick and others added 2 commits October 14, 2021 12:33
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@codecov-commenter
Copy link

codecov-commenter commented Oct 14, 2021

Codecov Report

Merging #5465 (ef087f7) into 1.5 (eb83ecb) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##                1.5    #5465   +/-   ##
=========================================
  Coverage     63.50%   63.50%           
  Complexity     1401     1401           
=========================================
  Files           195      195           
  Lines          4137     4137           
=========================================
  Hits           2627     2627           
  Misses         1510     1510           

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 eb83ecb...ef087f7. Read the comment docs.

Copy link
Contributor

@vitormicillo vitormicillo left a comment

Choose a reason for hiding this comment

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

Works perfect

@fletch3555 fletch3555 merged commit 4eb1735 into 1.5 Oct 18, 2021
@FromSi
Copy link

FromSi commented Nov 3, 2021

belongsToMany in BREAD

Error when entering the edit page in BREAD

photo_2021-11-03 18 25 42

@33Piter
Copy link

33Piter commented Nov 8, 2021

belongsToMany in BREAD

Error when entering the edit page in BREAD

photo_2021-11-03 18 25 42

Same problem here.

@emptynick
Copy link
Collaborator Author

emptynick commented Nov 9, 2021

That's not really a lot information to reproduce.
Can you guys figure out which query causes this?

Edit: Seems like this happens when you have an ID field in the pivot table - is there a reason to have this?

Edit 2: Fixed in 8c3c1dd

@33Piter
Copy link

33Piter commented Nov 10, 2021

That's not really a lot information to reproduce. Can you guys figure out which query causes this?

Edit: Seems like this happens when you have an ID field in the pivot table - is there a reason to have this?

Edit 2: Fixed in 8c3c1dd

Hello, @emptynick, thanks for the quick fix.

Came here to give more info, but you already found it.

That's exactly the scenario that causes the problem: I'm working on a project that have a pivot table with id column.

wimhendrikx added a commit to wimhendrikx/voyager that referenced this pull request Nov 29, 2021
* upstream/1.5:
  Hide download button if empty (thedevdojo#5487)
  Add Burmese Translations (thedevdojo#5488)
  Punctuation corrected (thedevdojo#5483)
  Update browse.blade.php (thedevdojo#5473)
  Automatic assets compilation (thedevdojo#5489)
  adding burmese lang (thedevdojo#5486)
  Fix belongs-to-many failing with ID on pivot table
  Bump path-parse from 1.0.6 to 1.0.7 (thedevdojo#5467)
  Bump tar from 6.1.0 to 6.1.11 (thedevdojo#5468)
  add ukrainian translations (thedevdojo#5448)
  Don't load all relationship results (thedevdojo#5465)
  Update Translatable.php (thedevdojo#5461)
@emptynick emptynick deleted the 1.5-relationship-ajax branch February 15, 2022 10:32
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.

None yet

6 participants