Bug #1850
Incompatible with e.g. redmine_local_avatars
100%
Description
extended_profile is incompatible with plugins that also extend the my_controller functionality. In my case I noticed that redmine_local_avatars’ account method would not be called any more, i.e. my users could not change their avatar. The reason is that extended_profile overwrites the account method and (possibly depending on initialisation order) thus overwrites other plugins.
I suggest to use alias_method_chain to implement the current 'extended_account’ method - this way you can always call the original function for the original code. E.g. I changed my_extended_controller_patch.rb to the following code and it still works and so does redmine_local_avatar:
require_dependency 'my_controller'
module MyExtendedControllerPatch
def self.included(base)
base.extend(ClassMethods)
base.send(:include, InstanceMethods)
base.class_eval do
unloadable
alias_method_chain :account, :extended
end
end
module ClassMethods
end
module InstanceMethods
def account_with_extended
if request.post?
@user = User.current
@profile = @user.profile
@pref = @user.pref
@user.profile.attributes = params[:profile]
if (@user.valid? & @user.valid_profile?) && @user.save
@user.profile.save
end
end
account_without_extended
end
end
end
There is also no need to worry too much about the code in the actual account method as it will just be called anyway. This maybe can make your checks for old redmine versions obsolete.
History
#1
Updated by Andriy Lesyuk over 12 years ago
Updated by - Status changed from New to Open
There is also no need to worry too much about the code in the actual account method as it will just be called anyway.
This is not exactly true... The initial version of the plugin did used alias_method_chain but I switched to rewriting it because errors were not shown for profile fields and field values got lost on errors (do not remember if this issue were related to) etc. This is due to redirect in account method.
See also: http://www.redmine.org/issues/8423
Anyway incompatibility issue exists and is important. Will try to find some solution (I can always add another check like I did for 1.0.x ).
Thank you very much for report!
#2
Updated by Andriy Lesyuk over 12 years ago
- Target version set to 0.0.3
#3
Updated by Andriy Lesyuk over 12 years ago
- Status changed from Open to In Progress
- % Done changed from 0 to 10
Weird... I do not find account change in Local Avatar! Could you please send me your copy of Local Avatar plugin? I tried to search but found several different versions...
#4
Updated by Daniel Seifert over 12 years ago
Updated by I used https://github.com/dseifert/redmine_local_avatars. It has lib/my_account_patch.rb with
alias_method_chain :account, :avatar
When going to my/account and uploading an avatar image, having extended_profile installed will cause the image to be lost. Using my above changes, the new avatar is successfully applied.
#5
Updated by Andriy Lesyuk over 12 years ago
- Status changed from In Progress to Incomplete
What if you make a mistake in e.g. “Company website”? I guess error message won’t be shown and/or the value you put won’t be placed into the field after you click “Save”... Am I right?
#6
Updated by Daniel Seifert over 12 years ago
No, however there is a caveat related to your implementation in that regards.
The point is, that upon hitting “Save” in the account settings, ROR will execute the account function. Now we have the problem that there are THREE account functions - the original one, the one from your plugin and the one from local_avatars (or any other plugin that extends the profile in some way). It should be necessary that all account functions are executed.
If you use alias_method, you hide the original account function. This is okay if yours is the only plugin as you copied the original functionality. However with another plugin like local_avatar that requires their own account function, you basically prevent this from being executed.
Using alias_method_chain, you can chain the functions together, so that after the first account function is executed, the next one is called. If there is an error at any of these methods, the call would abort. So you can still have checks and what not. However there is the issue that any of the account methods save the user model, but as long as the functionality is distinct this should not be a big issue (and definitely better than to prevent the other plugin to work at all).
#7
Updated by Andriy Lesyuk over 12 years ago
Weird... Tried myself (using my/account):
- Put “safgaga” into “Company website”
- Changed “Company” to “Test”
- Clicked "Save":
- Notice “Account was successfully updated” is displayed.
- No errors.
- “Company website” contains previous value (nothing in my case).
- “Company” holds previous value.
That’s what I told about...
#8
Updated by Andriy Lesyuk over 12 years ago
So this is perhaps the reason why I can’t include your changes... That is reasons are:
- I prefer better error handling over third-party plugins support;
- Redmine Local Avatar seems to have many versions and yours is only one of them and is not mainstream, right? So I can’t include code for Local Avatar as well... Unless you provide some variable which will indicate that it’s your plugin.
Anyway I have some idea how to fix this issue! The problem is that it’s not going to be fixed very soon!
Right now the only workaround is: http://projects.andriylesyuk.com/projects/redmine-profile/wiki/Wiki#Conflicts
Hope you don’t mind?
#9
Updated by Andriy Lesyuk over 12 years ago
- Target version deleted (
0.0.3)
#10
Updated by Andriy Lesyuk over 12 years ago
- Status changed from Incomplete to In Progress
- % Done changed from 10 to 50
#11
Updated by Andriy Lesyuk over 12 years ago
- % Done changed from 50 to 80
#12
Updated by Andriy Lesyuk over 12 years ago
- % Done changed from 80 to 100
#13
Updated by Daniel Seifert over 12 years ago
Andriy Lesyuk wrote:
So this is perhaps the reason why I can’t include your changes... That is reasons are:
- I prefer better error handling over third-party plugins support;
- Redmine Local Avatar seems to have many versions and yours is only one of them and is not mainstream, right?
That’s right.
So I can’t include code for Local Avatar as well... Unless you provide some variable which will indicate that it’s your plugin.
Anyway I have some idea how to fix this issue! The problem is that it’s not going to be fixed very soon!
Do your recent changes fix this (I haven’t had time to check it out yet)?
Right now the only workaround is: http://projects.andriylesyuk.com/projects/redmine-profile/wiki/Wiki#Conflicts
Hope you don’t mind?
I would prefer if you don’t name me as the author, as I’ve only made some adjustments but am not the original author (or even a significant contributor).
#14
Updated by Andriy Lesyuk over 12 years ago
Daniel Seifert wrote:
Andriy Lesyuk wrote:
So I can’t include code for Local Avatar as well... Unless you provide some variable which will indicate that it’s your plugin.
Anyway I have some idea how to fix this issue! The problem is that it’s not going to be fixed very soon!
Do your recent changes fix this (I haven’t had time to check it out yet)?
Yes! They do! That’s the solution I’m going to provide... But it’s not going to be a “fix” - it’s going to be another plugin you will need to migrate to...
Right now the only workaround is: http://projects.andriylesyuk.com/projects/redmine-profile/wiki/Wiki#Conflicts
Hope you don’t mind?I would prefer if you don’t name me as the author, as I’ve only made some adjustments but am not the original author (or even a significant contributor).
Fixed! Thanks.
#15
Updated by Andriy Lesyuk over 12 years ago
- Status changed from In Progress to Won't Fix