Skip to content
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

Added #14230: Add option to deactivate expected checkin notifications for users #14497

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

Support-AEI
Copy link

Description

This pull request adds a setting in the form of a checkbox to the general settings page. When this is checked, the user and admin will receive an email if the check-in of an asset is expected/overdue. However, if the checkbox is not activated, only the admin will receive the notification that the return of an asset is expected or overdue.
This change affects the php artisan snipe-it:expected-checkin command.

Language strings for German and US-English and the migration file for the database are included.

Fixes #14230

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.

  • Tested with activated checkbox and asset overdue for check-in
  • Tested without activated checkbox and asset overdue for check-in
  • Tested with activated checkbox and without asset overdue for check-in
  • Tested without activated checkbox and without asset overdue for check-in

Test Configuration:

  • PHP version: 8.2.7
  • MySQL version: Ver 15.1 Distrib 10.11.6-MariaDB
  • Webserver version: Apache/2.4.57
  • OS version: Debian GNU/Linux 12 (bookworm) Version 12.5

Checklist:

Copy link

what-the-diff bot commented Mar 26, 2024

PR Summary

  • Improved User Notification Process
    A condition was added in the server-side command file responsible for sending expected check-in alerts. This means notifications will only be sent out to users when specific notification parameters are met.

  • Updated Settings Control
    A new field representing user-level notification preferences was introduced in the Settings Controller. This affects how and when specific settings are updated.

  • Database Enhancements
    A new update file was created to introduce a new column in the database under the settings table. This represents user-level notification preferences, allowing those settings to be stored and retrieved when required.

  • Translation Additions
    In order to support various languages, new translations have been added for the newly introduced user-level notification preference field in both German and English.

  • User Interface Update
    The settings page has been updated with a new form element for user-level notification preferences. This enables users to choose their preference directly on the website itself.

@snipe
Copy link
Owner

snipe commented Mar 26, 2024

Thanks for this. Please remove the German translation file from this PR, as we use CrowdIn and the change you made will just be lost the next time we pull down translations.

I'm mostly loathe to add yet another setting option to be honest. We're working on revamping how notifications work, and I'm afraid this might just add more complexity down the line as we have to forklift existing functionality. I will discuss with the team internally.

@Support-AEI
Copy link
Author

No problem, the German language strings are removed.

@Glukose1
Copy link

Hey, that would be nice to have. Has a decision been made yet as to whether it will be implemented?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants