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

fix: edit url for block browser items #2533

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

Conversation

zeezo887
Copy link
Collaborator

Description

This resolves the issue where the edit URL of a block browser item fails to function properly when the browser name doesn't match the item module name.

Related Issues

Relates to #2278

@Tofandel
Copy link
Contributor

Tofandel commented Mar 21, 2024

Also just encountered this issue, the PR doesn't fix it entirely though

getModuleNameByModel() is not working properly if the model has a different name than the module (which it could have)

The fix is

    function getModuleNameByModel($model)
    {
        try {
            return TwillCapsules::getCapsuleForModel($model)->getModule();
        } catch (NoCapsuleFoundException) {
            return Str::plural(lcfirst(class_basename($model)));
        }
    }
    public function getCapsuleForModel(string|TwillModelContract $model): Capsule
    {
        if ($model instanceof TwillModelContract) {
            $model = get_class($model);
        }

        $model = class_basename($model);

        $capsule = $this->getRegisteredCapsules()->first(function (Capsule $capsule) use ($model) {
            return $capsule->getSingular() === $model;
        });

        if (!$capsule) {
            throw new NoCapsuleFoundException($model);
        }

        return $capsule;
    }

Comment on lines 252 to 263
$relatedType = $object->getRelation('pivot')->related_type;
$relation = str_contains($relatedType, '\\')
? getModuleNameByModel($relatedType)
: $relatedType;

return moduleRoute(
$relation,
config('twill.block_editor.browser_route_prefixes.' . $relation),
'edit',
$object->id
);
}
Copy link
Contributor

@Tofandel Tofandel Mar 21, 2024

Choose a reason for hiding this comment

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

Suggested change
$relatedType = $object->getRelation('pivot')->related_type;
$relation = str_contains($relatedType, '\\')
? getModuleNameByModel($relatedType)
: $relatedType;
return moduleRoute(
$relation,
config('twill.block_editor.browser_route_prefixes.' . $relation),
'edit',
$object->id
);
}
$module = getModuleNameByModel($object);
return moduleRoute(
$module,
config('twill.block_editor.browser_route_prefixes.' . $module),
'edit',
$object->id
);
}

@Tofandel
Copy link
Contributor

Tofandel commented Mar 21, 2024

Even with all this changes, the issue is still not fixed, newly added relations do get the correct link, but not the ones that were loaded when loading the page

image
image
image

@Tofandel
Copy link
Contributor

Tofandel commented Mar 21, 2024

There is another

'edit' => $relatedElement->adminEditUrl ?? moduleRoute(
      $moduleName ?? $relation,
      $routePrefix ?? '',
      'edit',
      $relatedElement->id
  ),

In handle browsers that needs to be replaced with 'edit' => $this->getAdminUrl($relatedElement)

After that it works correctly

@zeezo887
Copy link
Collaborator Author

@Tofandel Thanks for this! I didn't consider that a module name could be different from the model. Your suggested changes worked fine.

@Tofandel
Copy link
Contributor

Thanks, there is still this https://github.com/area17/twill/pull/2533/files#diff-3b134878262a01ad247d37cebcf578142d170ece67db85d1c31b8d0831c29c71R204-R208
To change according to my last comment

Copy link
Contributor

@Tofandel Tofandel left a comment

Choose a reason for hiding this comment

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

For backward compatibility

* @param $object
* @return mixed|string
*/
public function getAdminEditUrl($object): mixed
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public function getAdminEditUrl($object): mixed
public function getAdminEditUrl($object, ?string $routePrefix): mixed


return moduleRoute(
$module,
config('twill.block_editor.browser_route_prefixes.' . $module),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
config('twill.block_editor.browser_route_prefixes.' . $module),
$routePrefix ?? config('twill.block_editor.browser_route_prefixes.' . $module),

'edit',
$relatedElement->id
),
'edit' => $this->getAdminEditUrl($relatedElement),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'edit' => $this->getAdminEditUrl($relatedElement),
'edit' => $this->getAdminEditUrl($relatedElement, $routePrefix),

@ifox ifox linked an issue May 30, 2024 that may be closed by this pull request
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.

Block multi module browser bug
3 participants