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

Resolve tags by name instead of id #6

Merged
merged 3 commits into from
Nov 22, 2024

Conversation

Prokyonn
Copy link
Owner

@Prokyonn Prokyonn commented Nov 18, 2024

Q A
Bug fix? yes

What's in this PR?

Resolve tags by name instead of id.

Why?

Because the react admin always works with the name of the tag instead of the id.

To Do

  • Update tests

@Prokyonn Prokyonn force-pushed the enhancement/tag-resolver branch from fbf2c12 to e1c5cfd Compare November 18, 2024 07:36
@@ -32,11 +32,11 @@ public function __construct(

public function load(array $ids, ?string $locale, array $params = []): array
{
$result = $this->tagRepository->findBy(['id' => $ids]);
$result = $this->tagRepository->findBy(['name' => $ids]);

Choose a reason for hiding this comment

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

is this migrated correctly by the phpcr migration bundle? Currently tags are stored as ids in the excerpt in phpcr.

Copy link
Owner Author

@Prokyonn Prokyonn Nov 18, 2024

Choose a reason for hiding this comment

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

No it doesn't

But tbh I would like it more to save the id instead of the name, but then we would need to adjust it also on the FE 🤔

What would be the best way to go forward?


$mappedResult = [];
foreach ($result as $tag) {
$mappedResult[$tag->getId()] = $tag->getName();
$mappedResult[$tag->getName()] = $tag->getName();

Choose a reason for hiding this comment

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

Thinking about remove the loader if we just rereturn the names here.

@Prokyonn Prokyonn merged commit 3c20a61 into feature/content-resolver Nov 22, 2024
8 checks passed
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.

2 participants