Plugin code hooks

[desti]

From "manual"

Quote

When overriding a method, you MUST call the parent method so that ultimately you are inserting your code at the beginning or end of the method. This is necessary for allowing hooks on the same method to work.

 

Quote

When overriding a method, you MUST NOT copy code from the original method into your hook. This is necessary to ensure that your hook does not interfere with any bug fixes or changes made to the original class in future versions. Also, it is against the terms of the IPS Community Suite license to distribute any code within it.

I need to remove only one line from method, аll other code does not change. Real example - code hook for \IPS\core\modules\front\system\notifications, manage() method, remove code

\IPS\Db::i()->update( 'core_notifications', array( 'read_time' => time() ), array( '`member`=?', \IPS\Member::loggedIn()->member_id ) )

I can't call the parent method because update() will work.  
I can't not copy the code to hook because I need the functionality of the parent method.

How do you do this without violating these rules? 

[Adriano Faria]

1 hour ago, desti said:

How do you do this without violating these rules?

These rules are for marketplace submissions. You can override the whole function if you’re going to use it in your install only.

Be aware that overriding it will make any other hook in same function to not work.

[desti]

13 minutes ago, Adriano Faria said:

These rules are for marketplace submissions.

All only for my install. 
Ok. I'm creating another method, myManage() and copy manage() method code to it. Call with do=myManage. Formally it does not violate rules because i don't override method?

 

[DawPi]

2 minutes ago, desti said:

I'm creating another method, myManage() and copy manage() method code to it. Call with do=myManage. Formally it does not violate rules because i don't override method?

It's fine. 🙂

[Adriano Faria]

In other words, you’re copying their script and it’s a violation (again, marketplace rules). Anyway, you will be “bypassing” any change IPS may do in the manage().

[desti]

4 hours ago, Adriano Faria said:

Anyway, you will be “bypassing” any change IPS may do in the manage().

This is my problem, it is not so hard to look into the method of the next IPS release and correct plugin code.

But, returning to the first message: so I can 't make a plugin (without marketplace rules violation) that will just remove one line from the original code? 

[Adriano Faria]

1 minute ago, desti said:

But, returning to the first message: so I can 't make a plugin (without marketplace rules violation) that will just remove one line from the original code? 

Not sure you understood: if you will use it ONLY in your board and NOT submit to the marketplace, you can do it. You just need to be aware that doing this you will kill the chance of another hook works in same function/method.

[desti]

4 minutes ago, Adriano Faria said:

Not sure you understood

No, no, I 'm clear about my own board.

How would you solve this problem (remove one string from original code) if it need for a plugin for marketplace?

[desti]

I 'm not looking for a way to trick the rules, I just want to understand the principle :)

When you "press" on notification icon with active counter, all notification immediately set as read. My moderators ask me to change the algorithm: notification mark as read only after click on notification link. Ok, my plugin already written and work and (with very little probability) "i thought others would like it and decided to place it in the marketplace". But I can read and understand that my code breaks the rules. I can't call parent, ok, new method, but i can't change original code (look at manage() function), there everything is quite simple. What i've to do to avoid breaking the "don't copy" rule? 

[bfarber]

Unfortunately, sometimes there is no way to override a method properly without copying the code in it.

[desti]

And I'll be able to place my plugin in the marketplace? :) 

[bfarber]

According to the current marketplace guidelines, I don't think so (I don't approve submissions there though, so I can't say how lenient the review team is in situations like that).

I should point out that there's always more than one way to skin a cat too. I don't know exactly what your plugin is doing, but you may be able to add the code in a different manner that will have the same end effect but without overwriting the entire method.

[desti]

3 hours ago, bfarber said:

but you may be able to add the code in a different manner

Different manner for $table = new \IPS\Notification\Table(...) and another code that can 't be written in another way? Ok, no more question.

[sudo]

On 2/11/2020 at 1:41 PM, bfarber said:

I don't know exactly what your plugin is doing, but you may be able to add the code in a different manner that will have the same end effect but without overwriting the entire method

Basically rather than the notifications being marked as *notified* as such when you click the bell icon they will stay until the person has clicked each of the notifications in that area I think.

[desti]

sudo, that's right. 

Without violating the rules, I can only replace \IPS\Notification\Table(...) with \IPS\Db::i()->select(...) and my own template. 

Why create a framework and prevent it from being used?

 

[Daniel F]

I would suggest to override \IPS\Db::update and to check then the current app, module and controller for IPS\core\modules\front\system\notifications::manage to avoid that the update query is run.

Something like following pseudo code should work ( I haven't verified if all the names are correct, you'll have to confirm it yourself;)

public function update( $table, $set, $where='', $joins=array(), $limit=NULL, $flags=0 )
{
	if( !( $table === 'core_notifications' AND \IPS\Dispatcher::hasInstance() AND \IPS\Dispatcher::i()->controllerLocation === 'front' \IPS\Request::i()->module === 'system' AND \IPS\Request::i()->controller === 'notifications' AND !isset( \IPSUtf8\Request::i()->do )) )
	{
		return parent::update($table, $set, $where, $joins, $limit, $flags);
	}
}

 

[desti]

Daniel F - touché..

[bfarber]

Yeah without digging in to write the plugin myself it's hard for me to say specifically the best option - but the old adage "there's more than one way to skin a cat" applies.

[desti]

update() override works, but a hook with 4-5 condition check on the frequently used system function is a very bad style (IMHO). Maybe the review team doesn 't think so.

 

[Daniel F]

26 minutes ago, desti said:

update() override works, but a hook with 4-5 condition check on the frequently used system function is a very bad style (IMHO). Maybe the review team doesn 't think so.

 

I'm part of the MP review team;)

I won't disagree when somebody steps up and says that this is a bad style, but there's unfortunately not really an alternative which wouldn't break anything else, or which wouldn't cause any conflicts with other hooks.

[desti]

59 minutes ago, Daniel F said:

but there's unfortunately not really an alternative

I won't argue about the first rule, but the second one seems strange to me. We don't use code in other apps, everything inside IPS, why does it matter if the original code is used or not? The application developer must ensure that the version matches (not "tested with 4.2, 4.3, 4.4" as now, but "based on 4.2.0.7 version, on 4.4.9.2 version" and so on), this will be sufficient.

[DawPi]

2 hours ago, desti said:

We don't use code in other apps, everything inside IPS, why does it matter if the original code is used or not?

Do you know how it work? I doubt.

[desti]

DawPi, i joined this community a month ahead of you.  I know how this works.

[CodingJungle]

5 hours ago, desti said:

I won't argue about the first rule, but the second one seems strange to me. We don't use code in other apps, everything inside IPS, why does it matter if the original code is used or not? The application developer must ensure that the version matches (not "tested with 4.2, 4.3, 4.4" as now, but "based on 4.2.0.7 version, on 4.4.9.2 version" and so on), this will be sufficient.

it is how they protect their IP.  if they would allow you to copy entire sections of their code to remove 1 line, then they would have to allow anyone with search/replace the name "forum" to release an alternative forum's app, based almost entirely on their code. so it gives them a defense against IP theft. there is a third option, you could in theory rewrite the entire method to duplicate what they are doing, but you'd have the same problems, if they add new things in or you could accidentally introduce new bugs. its a rock and hard place sometimes, hopefully if the legends of a proper event system will come to fruition, that can alleviate these issues in the future, where a hook might not give us a path, but the event system does.  

[Makoto]

Indeed, there's a plugin I have that makes changes to the Elasticsearch API, but due to the way that class functions it can't be done without overriding and copying the entire method. So it can't be published in the MP.

It sucks, but sometimes that's just how it is. IPS has been improving in terms of modularity with every major release though it seems. 

An event system like @CodingJungle mentions would be wonderful.