Moodle

META: Cleanup of harcoded error() messages

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.9
  • Fix Version/s: 2.0
  • Component/s: General
  • Labels:
    None
  • Affected Branches:
    MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_20_STABLE

Description

There are thousands of places in Moodle where we are using the error() function to print out hardcoded strings.

In the long term, we should end with ZERO calls to this function, by using the print_error() on instead.

Here it's one plan that could be good to start handling this migration:

1) Move error() to deprecatelib. Just that. 1.9 and HEAD
2) Make print_error() autonomous so it won't rely on error() code anymore (will cause code duplication, I guess, but better in the long term.) 1.9 and HEAD
3) Under HEAD (not under 19_STABLE because it could be the plague!), in the deprecated error() function, add some DEBUG_DEVELOPER debugging() to inform developers. It will cause a lot of output (pain!)
4) Move all current uses of error(get_string('xxxx')) to a proper use of print_error() 1.9 and HEAD
5) Gradually, start creating error strings and changing error() calls to print_error() ones. Moodle 2.0 should be free of error() calls. HEAD only.

This is the overall plan. I think each step above could be a subtask of this. Please review it with MD. I think it could work (although once we do point 3 HEAD will be horrible until we'll have all the error() uses killed).

Ciao

Issue Links

This issue has a non-specific relationship to:
MDL-13217 Strings in lib/scroll_to_errors.js are hardcoded. Minor Closed
MDL-14082 Non-translatable error messages Minor Closed

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Dongsheng Cai added a comment -

I am working on step 4, I found code in /admin/mnet/peers.php:

error(get_string("invalidpubkey", 'mnet') . $errmsg ,'peers.php?step=update&hostid='.$mnet_peer->id);

After we change to print_error(), we cannot append string to the first parameter(which is a error code now), so I changed to:

print_error("invalidpubkey", 'mnet', 'peers.php?step=update&hostid='.$mnet_peer->id, $errmsg);

The problem is: $errmsg cannot print out, because in get_string(), the last parameter is useless, we just clean $a,
we didn't actually use it.

see moodlelib.php line 5158

Is this a bug?

Show
Dongsheng Cai added a comment - I am working on step 4, I found code in /admin/mnet/peers.php: error(get_string("invalidpubkey", 'mnet') . $errmsg ,'peers.php?step=update&hostid='.$mnet_peer->id); After we change to print_error(), we cannot append string to the first parameter(which is a error code now), so I changed to: print_error("invalidpubkey", 'mnet', 'peers.php?step=update&hostid='.$mnet_peer->id, $errmsg); The problem is: $errmsg cannot print out, because in get_string(), the last parameter is useless, we just clean $a, we didn't actually use it. see moodlelib.php line 5158 Is this a bug?
Hide
Permalink
Dongsheng Cai added a comment -

/var/www/dev# grep "print_error([\'\"]{1}[a-zA-Z0-9]\ {1}.?" -rl /var/www/dev | wc -l

297 need to be fixed.

Show
Dongsheng Cai added a comment - /var/www/dev# grep "print_error([\'\"]{1}[a-zA-Z0-9]\ {1}.?" -rl /var/www/dev | wc -l 297 need to be fixed.
Hide
Permalink
Dongsheng Cai added a comment -

Please reopen it if you find more unfixed print_error calls or hidden ones, TIA.

Show
Dongsheng Cai added a comment - Please reopen it if you find more unfixed print_error calls or hidden ones, TIA.
Hide
Permalink
Nicolas Martignoni added a comment -

The error string 'courserequestdisabled' in moodle.php lang file is not correctly called by print_error() at line 15 of course/request.php.

The call should be:

if (empty($CFG->enablecourserequests)) { print_error('courserequestdisabled', 'error'); }

and the string moved to error.php.

Moreover, these strings are hardcoded:

mod/chat/gui_header_js/insert.php, line 53: print_error('Could not insert a chat message!');
mod/quiz/report/responses/report.php, line 300: print_error('Could not load question options');

Show
Nicolas Martignoni added a comment - The error string 'courserequestdisabled' in moodle.php lang file is not correctly called by print_error() at line 15 of course/request.php. The call should be: if (empty($CFG->enablecourserequests)) { print_error('courserequestdisabled', 'error'); } and the string moved to error.php. Moreover, these strings are hardcoded: mod/chat/gui_header_js/insert.php, line 53: print_error('Could not insert a chat message!'); mod/quiz/report/responses/report.php, line 300: print_error('Could not load question options');
Hide
Permalink
Dongsheng Cai added a comment -

Thanks for reporting, Nicolas
print_error('courserequestdisabled');
print_error will look for lang string in error automatically if no module is indicated.

I will the others in MDL-14129

Show
Dongsheng Cai added a comment - Thanks for reporting, Nicolas print_error('courserequestdisabled'); print_error will look for lang string in error automatically if no module is indicated. I will the others in MDL-14129
Hide
Permalink
Nicolas Martignoni added a comment -

Thanks for you quick answer, Donsgsheng.

> print_error will look for lang string in error automatically if no module is indicated.

Yes, but observed that all calls to print_error() actually explicit the second argument, even if it's "error".

Further, the 'courserequestdisabled' is now in moodle,php, so it has to be moved to error.php.

Show
Nicolas Martignoni added a comment - Thanks for you quick answer, Donsgsheng. > print_error will look for lang string in error automatically if no module is indicated. Yes, but observed that all calls to print_error() actually explicit the second argument, even if it's "error". Further, the 'courserequestdisabled' is now in moodle,php, so it has to be moved to error.php.
Hide
Permalink
Dongsheng Cai added a comment -

Hi, Nicolas, I moved courserequestdisabled to error.php

Show
Dongsheng Cai added a comment - Hi, Nicolas, I moved courserequestdisabled to error.php

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.