Jump to content

PagesEditor.php throws error.


Inxentas
 Share

Recommended Posts

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

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

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

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

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 account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share

  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...