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.

Sign up for GitHub

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

Jump to bottom

FIX: Fixes #4376 partially added missing doc blocks. #8728

Closed
wants to merge 1 commit into from
Closed

FIX: Fixes #4376 partially added missing doc blocks. #8728

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions src/ORM/DataExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we need this sentence to be honest

*
* @return void
*/
public function onBeforeWrite()
{
}

/**
* Optional extension hook called by SilverStripe directly after a database write operation.
* Any logic expressed here, will be executed by SilverStripe.
*
* @return void
*/
public function onAfterWrite()
{
}

/**
* Optional extension hook called by SilverStripe directly before deleting a database write operation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

* Any logic expressed here, will be executed by SilverStripe.
*
* @return void
*/
public function onBeforeDelete()
{
}

/**
* Optional extension hook called by SilverStripe directly after deleting a database write operation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As with onBeforeDelete, I think the end of this sentence could be adjusted slightly

* Any logic expressed here, will be executed by SilverStripe.
*
* @return void
*/
public function onAfterDelete()
{
}

/**
* Optional extension hook called by SilverStripe directly during a dev/build operation.
* Any logic expressed here, will be executed by SilverStripe.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

*
* @return void
*/
public function requireDefaultRecords()
{
}
Expand All @@ -84,18 +114,37 @@ public function populateDefaults()
{
}


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/**
* 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
*/

public function can($member)
{
}

/**
* Whether or not the given $member is able to edit this decorated object.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* 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.

*
* @param Member $member
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to import the namespace for Member as well, up the top of the file: use SilverStripe\Security\Member;

* @return bool
*/
public function canEdit($member)
{
}

/**
* Whether or not the given $member is able to delete this decorated object.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* 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.

*
* @param Member $member
* @return bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 canDelete($member)
{
}

/**
* Whether or not the given $member is able to create this decorated object.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* 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.

*
* @param Member $member
* @return bool
*/
public function canCreate($member)
{
}
Expand Down