Skip to content

Conversation

@Coussecousse
Copy link
Contributor

@Coussecousse Coussecousse commented Oct 8, 2025

Hi !

I just added a health check box just as you requested (I think) in #6738 😄

Check if :

  • The mail DSN or the origin sender e-mail are not defined
  • If there are more than one active url, the global admin should be in it.

Screens :

  • Nothing is good 😞 :
Capture d’écran du 2025-10-08 14-45-38
  • URL is active and Admin is not register in it (:
Capture d’écran du 2025-10-08 14-21-04
  • Email is ok :
Capture d’écran du 2025-10-08 14-20-51
  • All is good 😄 :
Capture d’écran du 2025-10-08 14-20-22

⚠️ I don't know why, but mailer_from_email seems to be mixed up...
Proofs :
Capture d’écran du 2025-10-08 14-47-52
Capture d’écran du 2025-10-08 14-41-43

I noticed it when I was doing my test. I will investigate

erika added 4 commits October 8, 2025 13:47
Fix: Get active urls instead of all urls and get them from db If we don't get them from db, changes aren't apply Fix: Update mail settings check to handle 'null://null' as a valid empty value
@ywarnier
Copy link
Member

For mailer_from_email, we knew and updated it

Copy link
Member

@ywarnier ywarnier left a comment

Choose a reason for hiding this comment

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

Some improvements to be made later on, but overall OK. Thanks !

*
* @return array
*/
function api_get_active_urls()
Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding api_get_active_urls() here, you should define a similar method in src/CoreBundle/Helpers/AccessUrlHelpers.php
Then you could call it through:

$accessUrlHelper = Container::$container->get(AccessUrlHelper::class); $currentUrl = $accessUrlHelper->getCurrent(); 

Beware that these will return AccessUrl objects, which have to be called a bit differently than you code here.

foreach ($url_list as $u) {
if (!in_array($u->getId(), $my_user_url_list)) {
$url_string .= $u->getUrl() . '<br />';
if (!api_is_admin_in_all_active_urls()) {
Copy link
Member

Choose a reason for hiding this comment

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

The check should be on whether any user with GLOBAL_ADMIN privileges is registered in each URL. The current user account is not really important, but at least one global admin should be registered in each URL.

So the method api_is_admin_in_all_active_urls() should rather be called api_each_url_has_admin() or something like that. And ideally, this should be defined as a method of the AccessUrlHelper (see below).

@ywarnier ywarnier merged commit 74108d0 into chamilo:master Oct 28, 2025
0 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants