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

[2.x] Set position on multi selects #2167

Open
wants to merge 3 commits into
base: 2.x
Choose a base branch
from

Conversation

antonioribeiro
Copy link
Member

@antonioribeiro antonioribeiro commented Feb 27, 2023

Description

We have a model with a bunch of different tags:

Screenshot 2023-02-27 at 12 27 54

That are using multi-selects

$this->updateMultiSelect($object, $fields, 'countries');

One of our frontend pages needs to use the first tag of each of these fields. So I I tried to get the ordered:

public function countries()
{
    return $this->belongsToMany(Country::class, 'analysis_country', 'analysis_id', 'country_id')
                ->orderBy('analysis_country.position');
}

Just find that the position field is not being filled up, they are all null, so this PR is intended to do it automatically, unless there's already a position being set by the application.

Related Issues

@CLAassistant
Copy link

CLAassistant commented Feb 27, 2023

CLA assistant check
All committers have signed the CLA.

@antonioribeiro antonioribeiro changed the base branch from 3.x to 2.x February 27, 2023 11:34
@what-the-diff
Copy link

what-the-diff bot commented Feb 27, 2023

  • The updateMultiSelect method was changed to support the new pivot fields.
  • If an error occurs, maybe because the pivot table doesn't have a 'position' column, we will just keep the old implementation and sync without any extra parameters (the way it used to work).

@antonioribeiro
Copy link
Member Author

CLA assistant is in trouble... :(

Screenshot 2023-02-27 at 14 20 53

@ifox ifox changed the title Set position on multi selects [2.x] Set position on multi selects Mar 29, 2023
haringsrob added a commit that referenced this pull request Mar 30, 2023
ifox added a commit that referenced this pull request Feb 4, 2024
* #2167: Keep in mind order of multiselect.

* cleanup.

* Remove node-forge and update dependencies

* Update default compiled assets

---------

Co-authored-by: Quentin Renard <[email protected]>
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.

3 participants