FIX: Fixes #4376 partially added missing doc blocks. #8728
Conversation
There was a problem hiding this comment.
Thanks for the pull request! I've left a few suggestions, let me know what you think =)
Checklist (comparing against https://api.silverstripe.org/4/SilverStripe/ORM/DataExtension.html):
- onBeforeWrite
- onAfterWrite
- onBeforeDelete
- onAfterDelete
- requireDefaultRecords
- populateDefaults
- can
- canEdit
- canDelete
- canCreate
| public function onAfterWrite() | ||
| { | ||
| } | ||
|
|
||
| /** | ||
| * Optional extension hook called by SilverStripe directly before deleting a database write operation. |
There was a problem hiding this comment.
directly before deleting a database write operation
I think this might be a result of a copy/paste, would you mind updating it to say something like "directly before deleting a DataObject" or something like that?
| @@ -60,22 +60,52 @@ public function augmentWrite(&$manipulation) | |||
| { | |||
| } | |||
|
|
|||
| /** | |||
| * Optional extension hook called by SilverStripe directly before a database write operation. | |||
| * Any logic expressed here, will be executed by SilverStripe. | |||
There was a problem hiding this comment.
I'm not sure we need this sentence to be honest
| public function onBeforeDelete() | ||
| { | ||
| } | ||
|
|
||
| /** | ||
| * Optional extension hook called by SilverStripe directly after deleting a database write operation. |
There was a problem hiding this comment.
As with onBeforeDelete, I think the end of this sentence could be adjusted slightly
| public function onAfterDelete() | ||
| { | ||
| } | ||
|
|
||
| /** | ||
| * Optional extension hook called by SilverStripe directly during a dev/build operation. | ||
| * Any logic expressed here, will be executed by SilverStripe. |
There was a problem hiding this comment.
Here's the doc block from DataObject::requireDefaultRecords():
/**
* Add default records to database. This function is called whenever the
* database is built, after the database tables have all been created. Overload
* this to add default records when the database is built, but make sure you
* call parent::requireDefaultRecords().
*
* @uses DataExtension->requireDefaultRecords()
*/Perhaps you could use that and adjust it to communicate that it's run in an extension as opposed to directly on the object?
| @@ -84,18 +114,37 @@ public function populateDefaults() | |||
| { | |||
| } | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
| /** | |
| * Returns true if the member is allowed to do the given action. | |
| * See {@link extendedCan()} for a more versatile tri-state permission control. | |
| * | |
| * @param Member @member | |
| */ |
| /** | ||
| * Whether or not the given $member is able to edit this decorated object. | ||
| * | ||
| * @param Member $member |
There was a problem hiding this comment.
We need to import the namespace for Member as well, up the top of the file: use SilverStripe\Security\Member;
| * Whether or not the given $member is able to delete this decorated object. | ||
| * | ||
| * @param Member $member | ||
| * @return bool |
There was a problem hiding this comment.
Technically these methods return @return int|bool (see Permission::check()). The DataObject equivalents of these methods don't indicate that though, which is why it's a bit misleading
| public function canEdit($member) | ||
| { | ||
| } | ||
|
|
||
| /** | ||
| * Whether or not the given $member is able to delete this decorated object. |
There was a problem hiding this comment.
| * Whether or not the given $member is able to delete this decorated object. | |
| * Whether or not the given {@link Member} is able to delete the decorated object. |
| public function can($member) | ||
| { | ||
| } | ||
|
|
||
| /** | ||
| * Whether or not the given $member is able to edit this decorated object. |
There was a problem hiding this comment.
| * Whether or not the given $member is able to edit this decorated object. | |
| * Whether or not the given {@link Member} is able to edit the decorated object. |
| public function canDelete($member) | ||
| { | ||
| } | ||
|
|
||
| /** | ||
| * Whether or not the given $member is able to create this decorated object. |
There was a problem hiding this comment.
| * Whether or not the given $member is able to create this decorated object. | |
| * Whether or not the given {@link Member} is able to create the decorated object. |
|
Closing due to inactivity, please feel free to reopen the PR if you'd like to pick it up again, and thanks for contributing! |
added doc blocks
Fixes #4376