-
Notifications
You must be signed in to change notification settings - Fork 159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Custom setting additional date to customize the date display #463
base: MOODLE_311_STABLE
Are you sure you want to change the base?
Custom setting additional date to customize the date display #463
Conversation
…e the hard code way) mdjnelson#456
@@ -178,5 +178,46 @@ function xmldb_customcert_upgrade($oldversion) { | |||
upgrade_mod_savepoint(true, 2020110901, 'customcert'); | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change I did to change all old items in DB which are saved in format '1', '2'... to new format
@@ -305,11 +305,12 @@ public static function get_date_formats() { | |||
$date = 1530849658; | |||
|
|||
$suffix = self::get_ordinal_number_suffix(userdate($date, '%d')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes are the new way I change the format date. User will add new settings like "=%d# %B, %Y", which = is always the first character and '#' to present for 1st, 2nd, 3rd as handling by this function get_ordinal_number_suffix, and should add an intro for this symbol in the description too.
]); | ||
$DB->update_record('customcert_elements', $updateelement); | ||
}} | ||
$transaction->allow_commit();upgrade_mod_savepoint(true, 2021101100, 'customcert'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should be on a separate line.
@@ -178,5 +178,46 @@ function xmldb_customcert_upgrade($oldversion) { | |||
upgrade_mod_savepoint(true, 2020110901, 'customcert'); | |||
} | |||
|
|||
if ($oldversion < 2021101100) { | |||
$transaction = $DB->start_delegated_transaction(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't need to put this into a transaction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this improves performance as well as makes it more predictable
$records = $DB->get_records('customcert_elements', ['element' => 'date']); | ||
$total = count($records); | ||
$done = 0; | ||
$pbar = new progress_bar('mod_customcert_change_formatdate', 500, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for a progress bar as this is a quick change. I think all the code related to this progress bar can be removed. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a server had a very large number of custom certificates, it's possible to take a long time so I think it makes sense to add a progress bar here. What do you think?
Hi @mdjnelson, |
49b746c
to
8bc26b9
Compare
Hi @mdjnelson
I have created this pull request for issue #456.
Please take a look at it