Inxentas Posted October 16, 2023 Share Posted October 16, 2023 I'm having a little trouble deleting pages from a page class context. My approach might be screwy as well, but I'm open for critique. I am very much aware I'm doing no validaton yet. My page class PageAccountPage has an extended constructor that runs through a switch in order to centrally parse forms from it's template. Afterwards it sets a couple of variables, such as $this->addresses (which will store all addresses associated with the users account). This all works pretty well and ensures all form data is parsed before the page is rendered. However, I run into trouble when I delete a page in the switch under the deleteAddress case. I get the following error: Notice: Trying to get property 'id' of non-object in C:\wamp\www\mydomain.com\public_html\wire\core\PagesEditor.php on line 317 This implies the PagesEditor class is trying to grab a page by id that no longer existst due to it's deletion in the switch case. I assign $this->addresses after the switch, at the very end when everything should be parsed. I've also removed it to check whether it was causing the issue, but a little var_dump-ing points out that the error is thrown after the $a->delete line within the switch. A small note: the template has a prepend file set under Files named _webshop.php. Here is the full code: <?php namespace ProcessWire; /** * This is the class tied to the page-account template. */ class PageAccountPage extends WebshopPage { public $addresses; public function __construct(Template $tpl = null) { if($this->input->post->action) { $id = (int)$this->input->post->id; $title = (string)$this->sanitizer->text($this->input->post->title); $street = (string)$this->sanitizer->text($this->input->post->street); $house_nr = (string)$this->sanitizer->text($this->input->post->house_nr); $house_nr_suffix = (string)$this->sanitizer->text($this->input->post->house_nr_suffix); $zipcode = (string)$this->sanitizer->text($this->input->post->zipcode); $city = (string)$this->sanitizer->text($this->input->post->city); $phone = (string)$this->sanitizer->text($this->input->post->phone); $email = (string)$this->sanitizer->email($this->input->post->email); // switch actions switch($this->input->post->action) { case 'updatePassword': $password = (string)$page->input->post->password; $this->user->pass = $password; $this->user->save(); break; case 'loginAccount': $email = (string)$page->input->post->email; $password = (string)$page->input->post->password; $existingUser = $page->users->get("email=$email"); if($existingUser){ $page->session->login($existingUser->name, $password); } break; case 'createAccount': $name = (string)$page->input->post->email . '-' . time(); $email = (string)$page->input->post->email; $password = (string)$page->input->post->password; $existingUser = $page->users->find("name=$name"); if(!$existingUser){ $new = new User(); $new->of(false); $new->name = $name; $new->email = $email; $new->pass = $password; $new->save(); // TODO: Implement email validation and login through link? $page->session->login($new->name, $password); } else { // User exists, cannot remake the account. // TODO: consider a safe implementation of this case. } break; case 'updateAddress': // find page $a = $this->pages->get($id); $a->of(false); $a->title = $title; // populate fields $a->webshop_address_street = $street; $a->webshop_address_house_nr = $house_nr; $a->webshop_address_house_nr_suffix = $house_nr_suffix; $a->webshop_address_zipcode = $zipcode; $a->webshop_address_city = $city; $a->webshop_address_phone = $phone; $a->webshop_address_email = $email; // save page $a->save(); break; case 'deleteAddress': $a = $this->pages->get($id); $a->of(false); $a->delete(); break; case 'addAddress': // create page $new = new Page(); $new->of(false); $new->name = $title; $new->parent = $this->pages->find("template=page-addresses"); $new->template = $this->templates->get('page-address'); $new->name = $title; $new->title = $title; $new->save(); // populate fields $new->webshop_address_street = $street; $new->webshop_address_house_nr = $house_nr; $new->webshop_address_house_nr_suffix = $house_nr_suffix; $new->webshop_address_zipcode = $zipcode; $new->webshop_address_city = $city; $new->webshop_address_phone = $phone; $new->webshop_address_email = $email; // save page $new->save(); break; } } $parent = $this->pages->get("template=page-addresses"); $selector = "template=page-address,created_users_id=" . $this->user->id . ",parent=" . $parent->id; $this->addresses = $this->pages->find($selector); parent::__construct($tpl); } } Link to comment Share on other sites More sharing options...
Inxentas Posted October 16, 2023 Author Share Posted October 16, 2023 I've taken a look at the actual line in PagesEditor.php that throws this error. It's in the isDeleteable() method, the first line of this snippet: } else if($page->id === $this->wire()->page->id && $this->wire()->config->installedAfter('2019-04-04')) { $error = "it is the current page being viewed, try \$pages->trash() instead"; } Link to comment Share on other sites More sharing options...
BitPoet Posted October 16, 2023 Share Posted October 16, 2023 7 hours ago, Inxentas said: This implies the PagesEditor class is trying to grab a page by id that no longer existst due to it's deletion in the switch case. This also might be a red herring. The known fact is that either the $page parameter passed to isDeleteable() or the return of $this->wire()->page is null, and it might be the latter. From a gut feeling, this seems like an awful lot of action for a constructor, and it might make sense that the PageAccountPage is not fully wired with all variables before the constructor has finished. So I'd try moving the API logic out of the constructor and perhaps put it into ___loaded. Link to comment Share on other sites More sharing options...
Inxentas Posted October 17, 2023 Author Share Posted October 17, 2023 15 hours ago, BitPoet said: From a gut feeling, this seems like an awful lot of action for a constructor, and it might make sense that the PageAccountPage is not fully wired with all variables before the constructor has finished. So I'd try moving the API logic out of the constructor and perhaps put it into ___loaded. While this is a very unrefined script and I could always offload that line count to a method, I still think I'm abusing page class constructors a bit too much. Essentially treating them as MVC controllers. So when I got back this morning and had my What The Hell Was I Thinking experience I fully agree with that. However, I would like to handle incoming $input on a per-template basis so I'll move this code into some file and prepend it to the template. What I learned from this is that it's a hassle to get scope on API variables from these constructors. The intent here is to treat the actual template file as a "do whatever you want with it" file after install, so that's why I wanted to move parsing forms on the frontend anywhere but in the actual template file. Link to comment Share on other sites More sharing options...
Recommended Posts
Create an account or sign in to comment
You need to be a member in order to leave a comment
Create an account
Sign up for a new account in our community. It's easy!
Register a new accountSign in
Already have an account? Sign in here.
Sign In Now