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

Allow positioning media files #2694

Merged
merged 5 commits into from
Dec 4, 2024

Conversation

zeezo887
Copy link
Collaborator

@zeezo887 zeezo887 commented Nov 14, 2024

Description

I added a migration file to include a position column in the mediable and fileable tables, enabling the retrieval of items in a specific order based on their position.

Additionally, there is a bug in the $this->attachNew($records, $current, false) method when updating existing pivot records. The issue arises because:

$this->newPivotStatementForId($this->parseId($id))->update(
    $this->castAttributes($attributes)
);

public function newPivotStatementForId($id)
{
    return $this->newPivotQuery()->whereIn($this->relatedPivotKey, $this->parseIds($id));
}

This code uses the relatedPivotKey (such as media_id or file_id) to locate the record for updating. However, in our case, the IDs passed to this method are the IDs of the pivot table itself, not media_id or file_id. To resolve this, I overrode the attachNew method and filtered by the pivot table ID:

$updated = $this->newPivotQuery()->whereIn('id', $this->parseIds($id))->update(
    $this->castAttributes($attributes)
);

Related Issues

Fixes #2690 , #2689

@ifox ifox merged commit 709768e into area17:3.x Dec 4, 2024
8 checks passed
@LucaRed
Copy link
Contributor

LucaRed commented Dec 16, 2024

If I revert back to Twill 3.3.1, the positioning of media files used to work, without the need for this new migration. I'm just afraid there's something broken hiding somewhere.

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.

Cannot reorder images on A17\Twill\Services\Forms\Fields\Medias
3 participants