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.

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

[ticket/13981] Add events to capture avatar deletion or overwriting #3738

Merged
merged 6 commits into from
Aug 8, 2015

Conversation

javiexin
Copy link
Contributor

@javiexin javiexin commented Jul 3, 2015

PHPBB3-13981

An event to capture overwriting, and another to capture deletion.
Includes better error processing.

PHPBB3-13981
@@ -80,6 +80,8 @@ public function prepare_form($request, $template, $user, $row, &$error)
*/
public function process_form($request, $template, $user, $row, &$error)
{
global $phpbb_dispatcher;
Copy link
Member

Choose a reason for hiding this comment

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

The phpbb_dispatcher should be passed as a service and not be global.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

An event to capture overwriting, and another to capture deletion.
Includes better error processing. Replaced global by dependency injection.

PHPBB3-13981
* @param \phpbb\cache\driver\driver_interface $cache Cache driver
*/
public function __construct(\phpbb\config\config $config, $phpbb_root_path, $php_ext, \phpbb\path_helper $path_helper, \phpbb\mimetype\guesser $mimetype_guesser, \phpbb\cache\driver\driver_interface $cache = null)
public function __construct(\phpbb\config\config $config, $phpbb_root_path, $php_ext, \phpbb\path_helper $path_helper, \phpbb\mimetype\guesser $mimetype_guesser, \phpbb\cache\driver\driver_interface $dispatcher, \phpbb\cache\driver\driver_interface $cache = null)
Copy link
Member

Choose a reason for hiding this comment

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

The dispatcher is not \phpbb\cache\driver\driver_interface

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops... Copy&paste creep. Fixed.

An event to capture overwriting, and another to capture deletion.
Includes better error processing. Replaced global by dependency injection.
Fix typo.

PHPBB3-13981
@javiexin
Copy link
Contributor Author

Are the Travis and Bamboo errors relevant, or just caused because we have changed the constructor parameters, and therefore the tests do not match the new prototype?

@marc1706
Copy link
Member

No, you need to update the constructor(s) of the upload avatar in the tests. Travis is telling you which files you need to look in.

@javiexin
Copy link
Contributor Author

I don't know how to do that, I have never used Travis myself. I will look into it though.

An event to capture overwriting, and another to capture deletion.
Includes better error processing. Replaced global by dependency injection.
Fix Travis tests.

PHPBB3-13981
@javiexin
Copy link
Contributor Author

These tests failing seem unrelated to the submitted PR

    /home/travis/build/phpbb/phpbb/phpBB/phpbb/event/md_exporter.php:304
    /home/travis/build/phpbb/phpbb/phpBB/phpbb/event/md_exporter.php:145
    /home/travis/build/phpbb/phpbb/tests/event/md_exporter_test.php:121

@Zoddo
Copy link
Contributor

Zoddo commented Jul 23, 2015

@javiexin: Probably related to #3789

'row',
'error',
);
extract($this->dispatcher->trigger_event('core.avatar_driver_upload_overwrite_before', compact($vars)));
Copy link
Member

Choose a reason for hiding this comment

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

I'd like it more if you could change the name to core.avatar_driver_upload_move_file_before. It's not always overwriting a file so calling it that is somewhat not correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, change is coming.

An event to capture new avatar moving in place (and maybe overwriting
existing avatar), and another to capture deletion.  Includes better error
processing.  Rename event.

PHPBB3-13981
marc1706 added a commit to marc1706/phpbb that referenced this pull request Jul 27, 2015
[ticket/13981] Add events to capture avatar deletion or overwriting
marc1706 added a commit to marc1706/phpbb that referenced this pull request Aug 8, 2015
[ticket/13981] Add events to capture avatar deletion or overwriting
@marc1706 marc1706 merged commit c3d77ed into phpbb:3.1.x Aug 8, 2015
@javiexin javiexin deleted the ticket/13981 branch October 26, 2016 03:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants