femanager icon indicating copy to clipboard operation
femanager copied to clipboard

missing email AdminNotify after editing of user profile

Open ulrike-cosmoblonde opened this issue 1 year ago • 3 comments

This issue is related to #472

In my example I experienced the problem for the "edit" function. The notifyAdmin is not working due to the changes in: public/typo3conf/ext/femanager/Classes/Controller/AbstractController.php => updateAllConfirmed and in public/typo3conf/ext/femanager/Classes/Utility/ConfigurationUtility.php

The following problems exist:

  1. In both files the string 'edit/email/createUserNotify/notifyAdmin/receiver/email/value' should be changed into: 'edit/email/createUserNotify/notifyAdmin/receiver/email/value' I believe the /createUserNotify/ part was falsly added.

  2. The function ConfigurationUtility::notifyAdminAboutEdits should have an OR condition instead of an AND condition

if (self::getValue(
            'edit/notifyAdmin',
            $config
        ) && self::getValue(
            'edit/email/createUserNotify/notifyAdmin/receiver/email/value',
            $config
        )) {
            return true;
        }
  1. The function updateAllConfirmed in the AbstractController is missing a 2nd parameter in the ConfigurationUtility::getValue function calls
ConfigurationUtility::getValue('edit/email/createUserNotify/notifyAdmin/receiver/email/value') ??
ConfigurationUtility::getValue('edit/notifyAdmin'),

With those 3 adjustments the notifyAdmin should work as expected for updates.

Regards! Ulrike

ulrike-cosmoblonde avatar Apr 14 '23 12:04 ulrike-cosmoblonde

I think this is already fixed with https://github.com/in2code-de/femanager/pull/480/files

chludwig avatar Apr 21 '23 10:04 chludwig

By looking at the fix I can only see my 2nd suggested change implemented (changing the && into an ||). But my 1st and 3rd remarks are not implemented. As far as I can see, the string 'edit/email/createUserNotify/notifyAdmin/receiver/email/value' is not valid as the 'createUserNotify' should not be contained and the getValue function is still missing a 2nd parameter which is required.

ulrike-cosmoblonde avatar Apr 21 '23 10:04 ulrike-cosmoblonde

You're right please have a look at https://github.com/in2code-de/femanager/pull/496 and confirm if your tests were successful.

chludwig avatar Apr 21 '23 15:04 chludwig