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

Blog post preview in admin throws 500 #169

Closed
spaze opened this issue Jul 6, 2023 · 14 comments · Fixed by #170 or #190
Closed

Blog post preview in admin throws 500 #169

spaze opened this issue Jul 6, 2023 · 14 comments · Fixed by #170 or #190
Assignees

Comments

@spaze
Copy link
Owner

spaze commented Jul 6, 2023

Typed property MichalSpacekCz\Articles\Blog\BlogPost::$edits must not be accessed before initialization
File: .../Articles/Blog/BlogPostPreview.php:34

A regression introduced in #137 commit 18b1d2a, here:

-		$template->edits = $post->postId ? $this->blogPosts->getEdits($post->postId) : [];
+		$template->edits = $post->postId ? $post->edits : [];
@spaze spaze self-assigned this Jul 6, 2023
spaze added a commit that referenced this issue Jul 6, 2023
Another way to fix it would be to set the property to an empty array in `PostFormFactory::buildPost()`.

Close #169
@spaze spaze closed this as completed in #170 Jul 6, 2023
spaze added a commit that referenced this issue Jul 6, 2023
Another way to fix it would be to set the property to an empty array in `PostFormFactory::buildPost()`.

Close #169
@spaze
Copy link
Owner Author

spaze commented Jul 6, 2023

Reopening, this time it's Latte\RuntimeException: Missing template file '/srv/www/michalspacek.cz/site/app/Articles/Blog/../Www/Presenters/templates/Post/default.latte' Caused in #137 and 18b1d2a as well, by moving the \MichalSpacekCz\Articles\Blog\BlogPostPreview service one level down. Or caused by not having tests. Or both.

@spaze spaze reopened this Jul 6, 2023
@spaze spaze closed this as completed in 7c59931 Jul 6, 2023
@spaze
Copy link
Owner Author

spaze commented Jul 28, 2023

Long time no see. This time it's

Nette\InvalidArgumentException: Component with name 'articleHeaderIcons' does not exist. in /srv/www/michalspacek.cz/site/vendor/nette/component-model/src/ComponentModel/Container.php:149  @  https://admin.michalspacek.cz/blog/edit/30

Caused in #143, 091eafb.

@spaze spaze reopened this Jul 28, 2023
spaze added a commit that referenced this issue Jul 28, 2023
spaze added a commit that referenced this issue Jul 28, 2023
@lulco
Copy link
Contributor

lulco commented Jul 28, 2023

Are you able to create failing test case? We should found this kind of bugs with phpstan latte

@spaze
Copy link
Owner Author

spaze commented Jul 28, 2023

I may try to, need to understand the tests structure first though :-)

@lulco
Copy link
Contributor

lulco commented Jul 28, 2023

If you will not understand it, just try to describe it to me :) I tried to figure it out from the code, but I can't find the problem myself, because in this presenter templates there is no usage of this control

@spaze
Copy link
Owner Author

spaze commented Jul 29, 2023

Okay, thanks, I'll try to describe it first and then if you're going to write a test I'll use that as a reference next time.

First things first, this is going to be about the "Component with name 'articleHeaderIcons' does not exist" fixed in #190, I think that's what you're interested in this time, but I wanted to make sure we're on the same page. Maybe the root cause is the same as for "Missing template file" fixed in 7c59931.

I have a simple & stupid blog where you can write a blog post and click a "Preview" button and it will load the blog post into an iframe. The edit/preview form is created here:

protected function createComponentPost(): Form
{
return $this->postFormFactory->create(
function (BlogPost $post): never {
$this->blogPosts->add($post);
$this->flashMessage($this->texyFormatter->translate('messages.blog.admin.postadded', [$post->titleTexy, $this->link('edit', [$post->postId]), $post->href]));
$this->redirect('Blog:');
},
function (BlogPost $post): never {
$post->previousSlugTags = $this->post->slugTags ?? [];
$this->blogPosts->update($post);
$this->flashMessage($this->texyFormatter->translate('messages.blog.admin.postupdated', [$post->titleTexy, $this->link('edit', [$post->postId]), $post->href]));
$this->redirect('Blog:');
},
$this->template,

And this is the "Preview" click handler

->onClick[] = function (SubmitButton $button) use ($post, $template, $sendTemplate): void {
$newPost = $this->buildPost($this->formValues->getValues($button), $post?->postId);
$this->blogPostPreview->sendPreview($newPost, $template, $sendTemplate);

Note the $template variable on the last line:

$this->blogPostPreview->sendPreview($newPost, $template, $sendTemplate);

This is how it's passed to that method in the first code snippet above:

So we're kind of still in the same presenter where you don't see the articleHeaderIcons control anywhere in the according template. Well, that's because it's not there 😅

What I'll do in the sendPreview() method is this:

public function sendPreview(BlogPost $post, DefaultTemplate $template, callable $sendTemplate): never
{
$this->texyFormatter->disableCache();
$template->setFile(__DIR__ . '/../../Www/Presenters/templates/Post/default.latte');

I'll switch the template file for something else while technically still being in the old presenter.

And then the template response is sent with \Nette\Application\UI\Presenter::sendTemplate() here:

I want the preview to behave exactly like it should, meaning require authentication etc., but look exactly like it would look when viewed. It definitely can be written in a different way maybe but here we are 😅

So it could be described as a control that's used in a setFile()d template while technically being in the same presenter but the setFile() method is called in some other service.

I've realized there's a collector for finding setFile() calls but I don't really understand it very well, or at all, right now.

@lulco
Copy link
Contributor

lulco commented Jul 29, 2023

Uff 🤪😅
I'm not sure if we can solve this :)) it is little bit complicated O:-)

I have to read this again and again and again :D

but still it is hard to resolve some variables / paths which are set to template if it is sent to some other services etc. We have covered only some simple (štandard / most common) ways :)

@spaze
Copy link
Owner Author

spaze commented Jul 29, 2023

Yeah, I'll see if I can change it somehow.

@spaze
Copy link
Owner Author

spaze commented Aug 16, 2023

[2023-08-17 00-50-36] PHP Warning: Undefined variable $localeLinks in /srv/www/michalspacek.cz/site/temp/cache/latte/Presenters-templates-@layout.latte--0ded365051.php:159  @  https://admin.michalspacek.cz/blog/edit/78
[2023-08-17 00-50-36] PHP Warning: foreach() argument must be of type array|object, null given in /srv/www/michalspacek.cz/site/temp/cache/latte/Presenters-templates-@layout.latte--0ded365051.php:159  @  https://admin.michalspacek.cz/blog/edit/78

Fixed by #200

@lulco
Copy link
Contributor

lulco commented Aug 17, 2023

Do you think this could help? efabrica-team/phpstan-latte#424

@spaze
Copy link
Owner Author

spaze commented Aug 17, 2023

Do you think this could help? efabrica-team/phpstan-latte#424

Not sure it would help in this very specific case. Looking at the previous issues here, the problem is more that the whole template is not analyzed, don't get distracted that this time the problem was in @layout.latte 😁

@lulco
Copy link
Contributor

lulco commented Aug 17, 2023

Then maybe this one :) efabrica-team/phpstan-latte#425

But you are right, this is very specific case.

@spaze
Copy link
Owner Author

spaze commented Aug 17, 2023

Yeah, efabrica-team/phpstan-latte#425 looks more like it.

But I could also add a unit/something test that I think would also help, but I just didn't think/know I need one before I started using PHPStan Latte :-)

spaze added a commit that referenced this issue Aug 17, 2023
To avoid all blog post preview errors. This test successfully detects all the issues reported in #169.
spaze added a commit that referenced this issue Aug 17, 2023
To avoid all blog post preview errors. This test successfully detects all the issues reported in #169.
spaze added a commit that referenced this issue Aug 17, 2023
To avoid all blog post preview errors. This test successfully detects all the issues reported in #169.
spaze added a commit that referenced this issue Aug 17, 2023
To avoid all blog post preview errors. This test successfully detects all the issues reported in #169.
spaze added a commit that referenced this issue Aug 17, 2023
There are some hardcoded paths in the config that are hard to change and are needed for rendering the blog post template:
- SRI extension path to CSS, JS etc.
- SVG icons paths

These could be set in the test with
```php
$this->sriConfig->setLocalPrefix((object)['path' => __DIR__ . '/../../../public/www.michalspacek.cz']);
PrivateProperty::setValue($this->svgIconNodeFactory, 'iconsDir', __DIR__ . '/../../../node_modules/humbleicons/icons');
```
but the you get "Cannot modify readonly property Spaze\SvgIcons\Nodes\IconNodeFactory::$iconsDir"

Creating a custom template where neither SRI nor SVG icons are needed would be possible but then things like "Component with name 'articleHeaderIcons' does not exist" from #169 would not be tested.
spaze added a commit that referenced this issue Aug 17, 2023
…e repository checkout

There are some hardcoded paths in the config that are hard to change and are needed for rendering the blog post template:
- SRI extension path to CSS, JS etc.
- SVG icons paths

These could be set in the test with
```php
$this->sriConfig->setLocalPrefix((object)['path' => __DIR__ . '/../../../public/www.michalspacek.cz']);
PrivateProperty::setValue($this->svgIconNodeFactory, 'iconsDir', __DIR__ . '/../../../node_modules/humbleicons/icons');
```
but the you get "Cannot modify readonly property Spaze\SvgIcons\Nodes\IconNodeFactory::$iconsDir"

Creating a custom template where neither SRI nor SVG icons are needed would be possible but then things like "Component with name 'articleHeaderIcons' does not exist" from #169 would not be tested.
spaze added a commit that referenced this issue Aug 17, 2023
…e repository checkout

There are some hardcoded paths in the config that are hard to change and are needed for rendering the blog post template:
- SRI extension path to CSS, JS etc.
- SVG icons paths

These could be set in the test with
```php
$this->sriConfig->setLocalPrefix((object)['path' => __DIR__ . '/../../../public/www.michalspacek.cz']);
PrivateProperty::setValue($this->svgIconNodeFactory, 'iconsDir', __DIR__ . '/../../../node_modules/humbleicons/icons');
```
but the you get "Cannot modify readonly property Spaze\SvgIcons\Nodes\IconNodeFactory::$iconsDir"

Creating a custom template where neither SRI nor SVG icons are needed would be possible but then things like "Component with name 'articleHeaderIcons' does not exist" from #169 would not be tested.
spaze added a commit that referenced this issue Aug 17, 2023
…e repository checkout

There are some hardcoded paths in the config that are hard to change and are needed for rendering the blog post template:
- SRI extension path to CSS, JS etc.
- SVG icons paths

These could be set/overwritten in the test with

```php
$this->sriConfig->setLocalPrefix((object)['path' => __DIR__ . '/../../../public/www.michalspacek.cz']);
PrivateProperty::setValue($this->svgIconNodeFactory, 'iconsDir', __DIR__ . '/../../../node_modules/humbleicons/icons');
```
But then you get "Cannot modify readonly property Spaze\SvgIcons\Nodes\IconNodeFactory::$iconsDir"

Creating a custom template where neither SRI nor SVG icons are needed would be possible but then things like "Component with name 'articleHeaderIcons' does not exist" from #169 would not be tested.
spaze added a commit that referenced this issue Aug 18, 2023
To avoid all blog post preview errors. This test successfully detects all the issues reported in #169.
@spaze
Copy link
Owner Author

spaze commented Aug 18, 2023

I've added a unit/integration test in #203 that tests this functionality by just trying to render the template so there should be no more bugs in this part of the app (someone please remind me of this in a year or two).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants