TypeRocket / core

TypeRocket core source files where all the magic lives.
https://typerocket.com
36 stars 21 forks source link

Accessors bug #48

Closed zackphilipps closed 5 years ago

zackphilipps commented 5 years ago

Hey @kevindees,

https://github.com/TypeRocket/core/blame/master/src/Models/Model.php#L1550 https://github.com/TypeRocket/core/blame/master/src/Models/Model.php#L1565

These lines are causing accessors to not work if the property doesn't exist in the database. Accessors should still work even if the property does not exist in the database. These functions should simply return null here rather than throwing an error. Thoughts?

kevindees commented 5 years ago

@zackphilipps

Do you have some code that I could add to the unit tests to check on how you are using it and what the expected result should be?

zackphilipps commented 5 years ago

Sure!

public function getEditLinkProperty() {
  $page = new Page(Inflect::singularize($this->resource), 'edit', '');
  $str = '<a href="' . $page->getUrl(['route_id' => (int) $this->id]) . '">';
  $str .= $this->name;
  $str .= '</a>';
  return $str;
}

Then, $model->edit_link should be accessible.

kevindees commented 5 years ago

@zackphilipps

Master is updated. Let me know if that works for you.

zackphilipps commented 5 years ago

@kevindees This works for the Read/Update views but Create gets this error:

Fatal error: Uncaught InvalidArgumentException: name does not exist as a Model relationship in typerocket/vendor/typerocket/core/src/Models/Model.php:1548

"name" in this case is actually in the DB.

kevindees commented 5 years ago

@zackphilipps

Can you post a var dump of your model and its properties? I'm not sure where you are trying to access name from. Is name a public property of the model class?

Thanks, Kevin

zackphilipps commented 5 years ago

It’s just a field. The error is on $form->text('Name'). I can post more details later if need be

kevindees commented 5 years ago

@zackphilipps

I made the methods return null. Let me know if that worked for you.

zackphilipps commented 5 years ago

@kevindees It works! Is there anything else you think that needs to be done here, or is this an okay solution?

kevindees commented 5 years ago

I think it will work for now. My only thought is that it hides errors a little too good. Would be nice to have it fail with errors when nothing is there. I can add that back in later once I sort out how errors in the framework should be handled. There is no elegant error pattern I see at the moment that doesn't break WP.