learningworks / moodle-block_advnotifications Goto Github PK

Hi,

It would be great if it was possible to selectively display the block to members of specific cohorts.

Ricardo

https://docs.moodle.org/dev/Task_API

hi
i've done work to add rss support to advnotifications plugin. here's the respository URL:
https://github.com/brandeis-wmd/moodle-block_advnotifications

• the rss code was copied from blocks/rss_client
• it was integrated into the main object (might be better off done differently)
• the way it works is that if advnotifications "message" is a URL, code tries to retrieve feed and will display first description it finds in feed

i'm writing at this point to get make you aware of this work and see how it can be integrated into the plugin if desired.

so it's not really a bug, but this is a way to go forward. let me know, thx!

Add a basic WYSIWYG/HTML editor or allow basic HTML to be manually entered by more advanced users. This is to allow some additional basic customisation to message.

Likely to be added as optional feature (setting to enable/disable editor vs textbox). Entered HTML/content also to be filtered for security reasons.

This feature has been requested more than once on block's page at moodle.org.

Implement Privacy to support Moodle 3.5 fully and to meet GDPR requirements.

The default Moodle language pack "en" uses capital letters in first word of multi-word titles only. The block name should be "Advanced notifications" (not Notifications). This would help to achieve consistency with the rest of standard Moodle UI.

Currently a teacher (for example) can't add a new notification to a block. A common use-case may be for teachers to add notifications to their course - not globally.

I believe the capabilities are not set up correctly - likely the contexts. But also, finer granularity is needed. For example, a capability to only add non-global notifications, ideal for teachers, ensuring the notification is not shown in other contexts it wasn't intended for.

This issue was initially raised at https://moodle.org/plugins/block_advnotifications

https://docs.moodle.org/dev/Javascript_Modules

So we have few unit test failures, they do not influence the workflow itself but it would be nice to have it pass everything.

1. provider_testcase::test_metadata_provider with data set "block_advnotifications" ('block_advnotifications', 'block_advnotifications\privac...ovider')
   Failed asserting that false is true.

/moodle/privacy/tests/provider_test.php:131
/moodle/lib/phpunit/classes/advanced_testcase.php:80

This one seems to fail because lines 72 and 85 of blocks/advnotifications/classes/privacy/provider.php (in release 1.4.1) do not have "block_" prefix for table names.

Additional thing is that you print output in testcases which marks them as risky, but that is already nitpicking.

That's it, please add support for the ISO standard date format YYYY-MM-DD when used in the block settings for Date format. More: https://www.iso.org/iso-8601-date-and-time-format.html

Thanks

Upgrading from 1.3.9 to 1.3.11, on the upgradesettings.php page see the following warning:

"Warning: key() expects parameter 1 to be array, string given in [path]/moodle/blocks/advnotifications/settings.php on line 101"

Also am seeing it on the Site Administration page (/moodle/admin/search.php). I have Debug messages set to DEVELOPER and Display debug messages to Yes on my development site where I am seeing the warning. The message goes away if I turn Display debug messages off.

The plugin seems to function as expected but perhaps the warning should be addressed?

Versions:
Moodle 3.5.14
PHP 7.2.24

When I create an instance of this block on the front page and set Page contexts to "Display throughout entire site", and if the setting block_advnotifications | enable is set to true, then both the forgotten password link on the front page and the forced password reset option on user accounts have errors. The former redirects back to the login page, and the latter gives an error ERR_TOO_MANY_REDIRECTS.

Note: I'm using the Essential theme and Moodle 3.4, some testing seems to indicate this error may only be with the Essential theme.

Renderers should not ever touch the database. The actual logic of obtaining data to render should stand outside the output rendering logic.

Additionally, SQL queries should not be called in loops. The number of SQL queries should remain constant regardless the number of notifications to display. Ideally you should be able to fetch all data in one query and pass them to the renderer.

From Moodle.org

"Hi.

I'm getting the message "Installation aborted due to validation failure" when I try to update.

Moodle 3.5.1+ (Build: 20180713)

Regards"

It is a common pattern to set the content property during the first call and avoid further processing (and repeated DB calls and everything) via returning the property early. Example: https://github.com/moodle/moodle/blob/master/blocks/html/block_html.php#L56-L58

Would it be possible to use language strings in the values of the Types (warning, success, etc.)? In the table view all the English language strings are translatable, except this one. (PS: maybe use in that case strotolower php function to de-capitalize the first letter).

Recommendation from moodle.org.

Currently, if the user does not select a from and to date it is displayed as 01/01/1970. Rather, set it to display as a dash (-) as an indication of no date chosen.

The textarea for the message field is of the same (min-)width as the title field. Although there is an option to enlarge the message field, maybe it's an idea to enlarge the min-width because a message is usually (much) larger than the title field. Visually this would be more attractive, I think.

Moodle 3.8.5+
PostgreSQL 12
PHP 7.4
Ubuntu 20.04

Debug info: ERROR: value "32768" is out of range for type smallint
UPDATE mdl_block_advnotificationsdissed SET seen = $1 WHERE id=$2
[array (
'seen' => 32768,
0 => '1',
)]
Error code: dmlwriteexception
Stack trace:

line 489 of /lib/dml/moodle_database.php: dml_write_exception thrown
line 259 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->query_end()
line 1184 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end()
line 1217 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->update_record_raw()
line 99 of /blocks/advnotifications/locallib.php: call to pgsql_native_moodle_database->update_record()
line 66 of /blocks/advnotifications/block_advnotifications.php: call to prep_notifications()
line 341 of /blocks/moodleblock.class.php: call to block_advnotifications->get_content()
line 235 of /blocks/moodleblock.class.php: call to block_base->formatted_contents()
line 91 of /blocks/advnotifications/block_advnotifications.php: call to block_base->get_content_for_output()
line 1181 of /lib/blocklib.php: call to block_advnotifications->get_content_for_output()
line 1239 of /lib/blocklib.php: call to block_manager->create_block_contents()
line 374 of /lib/blocklib.php: call to block_manager->ensure_content_created()
line 3920 of /lib/outputrenderers.php: call to block_manager->region_has_content()
line 49 of /theme/fordson/layout/frontpage.php: call to core_renderer->blocks()
line 1374 of /lib/outputrenderers.php: call to include()
line 1304 of /lib/outputrenderers.php: call to core_renderer->render_page_layout()
line 100 of /index.php: call to core_renderer->header()

I expect almost all users will stick to the default alert colours and icons but we have had to change them. However they are hard coded and do not allow theme overrides.

I have attached a small patch to fix this.
block_advnotifications.diff.txt

I useful option for further development (of this very nice plugin!) could be the option to show specific notifications to different user- or sitegroups. Especially for larger Moodle-sites this would be a handy option.

JavaScript error shown in title is thrown when using the Advanced Notification block, whether or not there is an active notification.

Hello,
When on the "Manage Notifications" page, I cannot find a way to adjust how the date is displayed. Currently, it's in DD-MM-YYYY and I don't see a language string to change it to MM-DD-YYYY. Is this something that can be adjusted?

When I make a new notification and put the dates into the date fields in the dd/mm/yyyy placeholder format, the dates are incorrect saved as: mm/dd/yyyy. And I have to edit the dates afterwards to solve this. When I fill it in as yyyy/mm/dd, the date fields are saved in the format dd/mm/yyyy, which is in my case the correct way to display the notification on the proper dates. I'm using Moodle 3.6 with a child theme of Boost theme. What could be the problem and solution?

There are 5 styles to choose from when setting up a notification: Info, Success, Warning, Danger, Announcement

When Announcement is selected, it is identical to Info (according to the CSS when viewing the browser tools).

Is it possible to make "announcement" a separate class? I'd love to make it a nice purple on my site.

This will allow the content of the message to support multiple languages on a single Moodle site (since a notification's Title/Message won't be Moodle langstrings).