nextcloud / announcementcenter

📢 Announcement Center for Nextcloud
https://apps.nextcloud.com/apps/announcementcenter
GNU Affero General Public License v3.0
39 stars 24 forks source link

Schedule publishing and deletion time #767

Closed mwinkens closed 4 months ago

mwinkens commented 8 months ago

Being able to schedule announcements is a wished feature from the community, this is completely optional of course

Images: App view: ![Bildschirmfoto vom 2024-03-11 14-02-56](https://github.com/nextcloud/announcementcenter/assets/104770531/372f2df0-3430-48ef-acc3-25fa7442a8ae) Test user: (he only got the notification of a soon to be deleted announcement) ![Bildschirmfoto vom 2024-03-11 14-13-06](https://github.com/nextcloud/announcementcenter/assets/104770531/82eb1e5c-494d-49d9-a514-565b0609175a)

Features:

closes #187 closes #677

possible milestone for #385

nickvergessen commented 8 months ago

I invited you to our org, so you can send the PR from with in, so it also automatically runs CI

mwinkens commented 8 months ago

I invited you to our org, so you can send the PR from with in, so it also automatically runs CI

Thank you! I hope I didn't cause too much traffic

mwinkens commented 8 months ago

@nickvergessen currently the background job is not being executed and I don't know why. When I re-enable the app, the job is added but as soon as you want to execute it, it gets deleted :thinking:

I am pretty sure I set it up as described in the developer documentation, any ideas? I already set the loglevel to debug and added logging to the Service, but I didn't get any output.

mysql> select * from oc_jobs where class LIKE '%nnouncement%' ;
+-----+-------------------------------------------------+----------+------------+--------------+-------------+--------------------+----------------------------------+----------------+
| id  | class                                           | argument | last_run   | last_checked | reserved_at | execution_duration | argument_hash                    | time_sensitive |
+-----+-------------------------------------------------+----------+------------+--------------+-------------+--------------------+----------------------------------+----------------+
|  17 | OCA\NextcloudAnnouncements\Cron\Crawler         | null     | 1710231001 |   1710246001 |           0 |                  0 | 37a6259cc0c1dae299a7866489dff0bd |              1 |
| 195 | OCA\AnnouncementCenter\AnnouncementSchedulerJob | null     |          0 |   1710246080 |           0 |                  0 | 37a6259cc0c1dae299a7866489dff0bd |              1 |
+-----+-------------------------------------------------+----------+------------+--------------+-------------+--------------------+----------------------------------+----------------+
sudo -u www-data php occ background-job:list | grep nnouncement
| 17  | OCA\NextcloudAnnouncements\Cron\Crawler                         | 2024-03-12T08:10:01+00:00 | null     |

https://docs.nextcloud.com/server/latest/developer_manual/basics/backgroundjobs.html#writing-a-background-job


Edit: I found out, that the DependencyInjection is not working/not setup, because the services are not registered. This is also true for your custom Nofitier. I will implement this

https://docs.nextcloud.com/server/latest/developer_manual/basics/dependency_injection.html

nickvergessen commented 8 months ago

Edit: I found out, that the DependencyInjection is not working/not setup, because the services are not registered. This is also true for your custom Nofitier. I will implement this

https://docs.nextcloud.com/server/latest/developer_manual/basics/dependency_injection.html

This should only be needed in very rare circumstances and in fact in the code I see no reason why it would be needed in this case

mwinkens commented 8 months ago

Edit: I found out, that the DependencyInjection is not working/not setup, because the services are not registered. This is also true for your custom Nofitier. I will implement this https://docs.nextcloud.com/server/latest/developer_manual/basics/dependency_injection.html

This should only be needed in very rare circumstances and in fact in the code I see no reason why it would be needed in this case

Should I remove this again? I noticed, that at least the Notifier needed to be added there, because I got a warning

{"reqId":"B2lvZSwcW2eEm61Gr5lJ","level":3,"time":"2024-03-12T15:41:04+00:00","remoteAddr":"127.0.0.1","user":"nextcloud27","app":"notifications","method":"GET","url":"/ocs/v2.php/apps/notifications/api/v2/notifications","message":"Failed to load notification notifier class: OCA\\AnnouncementCenter\\Notification\\Notifier","userAgent":"Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:122.0) Gecko/20100101 Firefox/122.0","version":"28.0.1.1","exception":{"Exception":"OCP\\AppFramework\\QueryException","Message":"Could not resolve OCA\\AnnouncementCenter\\AppInfo\\IConfig! Class \"OCA\\AnnouncementCenter\\AppInfo\\IConfig\" does not exist","Code":0,"Trace":[{"function":"OC\\AppFramework\\Utility\\{closure}","class":"OC\\AppFramework\\Utility\\SimpleContainer","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/var/www/nextcloud28/lib/private/AppFramework/Utility/SimpleContainer.php","line":83,"function":"array_map"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/Utility/SimpleContainer.php","line":128,"function":"buildClass","class":"OC\\AppFramework\\Utility\\SimpleContainer","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/Utility/SimpleContainer.php","line":146,"function":"resolve","class":"OC\\AppFramework\\Utility\\SimpleContainer","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/DependencyInjection/DIContainer.php","line":468,"function":"query","class":"OC\\AppFramework\\Utility\\SimpleContainer","type":"->"},{"file":"/var/www/nextcloud28/lib/private/ServerContainer.php","line":155,"function":"queryNoFallback","class":"OC\\AppFramework\\DependencyInjection\\DIContainer","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/Utility/SimpleContainer.php","line":64,"function":"query","class":"OC\\ServerContainer","type":"->"},{"file":"/var/www/nextcloud28/lib/private/Notification/Manager.php","line":173,"function":"get","class":"OC\\AppFramework\\Utility\\SimpleContainer","type":"->"},{"file":"/var/www/nextcloud28/lib/private/Notification/Manager.php","line":364,"function":"getNotifiers","class":"OC\\Notification\\Manager","type":"->"},{"file":"/var/www/nextcloud28/apps/notifications/lib/Controller/EndpointController.php","line":118,"function":"prepare","class":"OC\\Notification\\Manager","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/Http/Dispatcher.php","line":230,"function":"listNotifications","class":"OCA\\Notifications\\Controller\\EndpointController","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/Http/Dispatcher.php","line":137,"function":"executeController","class":"OC\\AppFramework\\Http\\Dispatcher","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/App.php","line":184,"function":"dispatch","class":"OC\\AppFramework\\Http\\Dispatcher","type":"->"},{"file":"/var/www/nextcloud28/lib/private/Route/Router.php","line":315,"function":"main","class":"OC\\AppFramework\\App","type":"::"},{"file":"/var/www/nextcloud28/ocs/v1.php","line":65,"function":"match","class":"OC\\Route\\Router","type":"->"},{"file":"/var/www/nextcloud28/ocs/v2.php","line":23,"args":["/var/www/nextcloud28/ocs/v1.php"],"function":"require_once"}],"File":"/var/www/nextcloud28/lib/private/AppFramework/Utility/SimpleContainer.php","Line":114,"Previous":{"Exception":"OC\\AppFramework\\Utility\\QueryNotFoundException","Message":"Could not resolve OCA\\AnnouncementCenter\\AppInfo\\IConfig! Class \"OCA\\AnnouncementCenter\\AppInfo\\IConfig\" does not exist","Code":0,"Trace":[{"file":"/var/www/nextcloud28/lib/private/AppFramework/Utility/SimpleContainer.php","line":146,"function":"resolve","class":"OC\\AppFramework\\Utility\\SimpleContainer","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/DependencyInjection/DIContainer.php","line":468,"function":"query","class":"OC\\AppFramework\\Utility\\SimpleContainer","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/DependencyInjection/DIContainer.php","line":440,"function":"queryNoFallback","class":"OC\\AppFramework\\DependencyInjection\\DIContainer","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/Utility/SimpleContainer.php","line":64,"function":"query","class":"OC\\AppFramework\\DependencyInjection\\DIContainer","type":"->"},{"file":"/home/marvin/workspace/announcementcenter/lib/AppInfo/Application.php","line":76,"function":"get","class":"OC\\AppFramework\\Utility\\SimpleContainer","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/Utility/SimpleContainer.php","line":175,"function":"OCA\\AnnouncementCenter\\AppInfo\\{closure}","class":"OCA\\AnnouncementCenter\\AppInfo\\Application","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/var/www/nextcloud28/3rdparty/pimple/pimple/src/Pimple/Container.php","line":122,"function":"OC\\AppFramework\\Utility\\{closure}","class":"OC\\AppFramework\\Utility\\SimpleContainer","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/var/www/nextcloud28/lib/private/AppFramework/Utility/SimpleContainer.php","line":142,"function":"offsetGet","class":"Pimple\\Container","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/DependencyInjection/DIContainer.php","line":462,"function":"query","class":"OC\\AppFramework\\Utility\\SimpleContainer","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/DependencyInjection/DIContainer.php","line":440,"function":"queryNoFallback","class":"OC\\AppFramework\\DependencyInjection\\DIContainer","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/Utility/SimpleContainer.php","line":96,"function":"query","class":"OC\\AppFramework\\DependencyInjection\\DIContainer","type":"->"},{"function":"OC\\AppFramework\\Utility\\{closure}","class":"OC\\AppFramework\\Utility\\SimpleContainer","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/var/www/nextcloud28/lib/private/AppFramework/Utility/SimpleContainer.php","line":83,"function":"array_map"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/Utility/SimpleContainer.php","line":128,"function":"buildClass","class":"OC\\AppFramework\\Utility\\SimpleContainer","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/Utility/SimpleContainer.php","line":146,"function":"resolve","class":"OC\\AppFramework\\Utility\\SimpleContainer","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/DependencyInjection/DIContainer.php","line":468,"function":"query","class":"OC\\AppFramework\\Utility\\SimpleContainer","type":"->"},{"file":"/var/www/nextcloud28/lib/private/ServerContainer.php","line":155,"function":"queryNoFallback","class":"OC\\AppFramework\\DependencyInjection\\DIContainer","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/Utility/SimpleContainer.php","line":64,"function":"query","class":"OC\\ServerContainer","type":"->"},{"file":"/var/www/nextcloud28/lib/private/Notification/Manager.php","line":173,"function":"get","class":"OC\\AppFramework\\Utility\\SimpleContainer","type":"->"},{"file":"/var/www/nextcloud28/lib/private/Notification/Manager.php","line":364,"function":"getNotifiers","class":"OC\\Notification\\Manager","type":"->"},{"file":"/var/www/nextcloud28/apps/notifications/lib/Controller/EndpointController.php","line":118,"function":"prepare","class":"OC\\Notification\\Manager","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/Http/Dispatcher.php","line":230,"function":"listNotifications","class":"OCA\\Notifications\\Controller\\EndpointController","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/Http/Dispatcher.php","line":137,"function":"executeController","class":"OC\\AppFramework\\Http\\Dispatcher","type":"->"},{"file":"/var/www/nextcloud28/lib/private/AppFramework/App.php","line":184,"function":"dispatch","class":"OC\\AppFramework\\Http\\Dispatcher","type":"->"},{"file":"/var/www/nextcloud28/lib/private/Route/Router.php","line":315,"function":"main","class":"OC\\AppFramework\\App","type":"::"},{"file":"/var/www/nextcloud28/ocs/v1.php","line":65,"function":"match","class":"OC\\Route\\Router","type":"->"},{"file":"/var/www/nextcloud28/ocs/v2.php","line":23,"args":["/var/www/nextcloud28/ocs/v1.php"],"function":"require_once"}],"File":"/var/www/nextcloud28/lib/private/AppFramework/Utility/SimpleContainer.php","Line":135},"message":"Failed to load notification notifier class: OCA\\AnnouncementCenter\\Notification\\Notifier","exception":{},"CustomMessage":"Failed to load notification notifier class: OCA\\AnnouncementCenter\\Notification\\Notifier"}}
nickvergessen commented 8 months ago

Should I remove this again? I noticed, that at least the Notifier needed to be added there, because I got a warning

Yeah remove it again. It seems there is an import of use OCP\IConfig; missing somewhere instead.

mwinkens commented 8 months ago

@nickvergessen this features are now finished and in a usable state, I tested it manually since tests are still missing, scheduled deletion and scheduled announcing works.

Some notes/caveats, do you which for some of these things to be implemented here?:

Next thing on my list is to add tests and call it a day

mwinkens commented 8 months ago

Waiting for feedback :partying_face:

mwinkens commented 8 months ago

@nickvergessen Thanks for your review, I implemented everything! I tested this feature again manually, and everything (new) was working as expected

nickvergessen commented 8 months ago

What about the two points from the description:

Want to address them in follow ups? Then we could potentially merge this PR here and release it already.

mwinkens commented 8 months ago

What about the two points from the description:

* Add button to schedule deletion for existing announcements

* Add publish button for unannounced announcements

Want to address them in follow ups? Then we could potentially merge this PR here and release it already.

I would address them in follow ups, because I already have a follow up (the banners) and the scheduled deletion would require some extra work, and should maybe be handled by a general edit option, which is still missing. Same applies if you want an other publish time

The direct publishing requires an extra route, but I'll add a follow up for this

@nickvergessen I let you resolve the conflict here, because it's only in the version, do you wish this to be ~6.8.1~(published on app store), 6.8.2 or 6.9.0?

github-actions[bot] commented 8 months ago

Hello there, Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

mwinkens commented 5 months ago

Hello @nickvergessen, are you okay with these changes?

nickvergessen commented 5 months ago

Sorry, I'm heavily overloaded atm. I hope I find the time soon to have a look.

mwinkens commented 4 months ago

you want to change to that? I think it'd be better

Maybe we should use https://www.php.net/manual/en/function.json-encode.php here, what do you think @nickvergessen ?

nickvergessen commented 4 months ago

Maybe we should use https://www.php.net/manual/en/function.json-encode.php here, what do you think

I'd appriciate this to avoid any possibility to break with "strange" characters in group names

nickvergessen commented 4 months ago

So just to summarize, my proposal would be:

Raise issues for follow ups:

And then we can merge this and include it in the next RC which I need to publish for 30 compatibility first half of next week

mwinkens commented 4 months ago

@nickvergessen I finished everything on your list and opened an issue for the followup. I also reset the package files, however I'd like you to know, that for the next release I'd upgrade the packages: 10 vulnerabilities (5 moderate, 5 critical) from npm