๐Ÿ‡ฎ๐Ÿ‡ณIndia @pirtpal_singh

Ludhana
Account created on 4 April 2025, about 1 month ago
#

Recent comments

๐Ÿ‡ฎ๐Ÿ‡ณIndia pirtpal_singh Ludhana

Thank you for reporting the issue.

This problem occurred because the user_role_names() function was incorrectly referenced. I have fixed it by using the correct method to load user roles within the SendNotificationForm class.

The fix has been committed, and the error should no longer occur when selecting Specific Roles in the Send To dropdown.

Please update the module to the latest version and let me know if you face any further issues. Thanks again for helping improve the module!

๐Ÿ‡ฎ๐Ÿ‡ณIndia pirtpal_singh Ludhana

Thank you for reporting this.

This issue has already been reported and discussed in the following thread:

https://www.drupal.org/project/firebase_ui/issues/3520771#comment-16085835 ๐Ÿ› file upload not working Active

I'm closing this issue as a duplicate to keep the conversation centralized. Please follow the original thread for updates and resolution progress.

Thanks again for your contribution and support in improving the module!

๐Ÿ‡ฎ๐Ÿ‡ณIndia pirtpal_singh Ludhana

Thank you for reporting these issues and sharing your suggestions.

Latest Updates

1. JSON File Upload in Drupal 11

This issue has been resolved. The module was originally tested up to Drupal 10, and the file upload validator used was incompatible with Drupal 11. As suggested, the upload validator has been updated to:

'#upload_validators' => [
  'FileExtension' => ['json'],
],

2. Support for Multiple Application IDs

Currently, the form supports only a single application ID. I recognize the need to accommodate both iOS and Android app IDs and will consider this for a future release.

3. Sending Notifications to Multiple Devices

Firebase doesnโ€™t allow sending to an array of tokens in one request via the current API, so notifications are queued and sent individually. However, Iโ€™m actively researching better ways to handle high-volume delivery more efficiently.

4. Multiple appId Values in Config Form

This is a valid feature request. I will explore adding support for multiple values in the appId field in future updates.

Thanks again for your detailed feedback โ€” it's incredibly helpful in improving the moduleโ€™s quality and usefulness.

๐Ÿ‡ฎ๐Ÿ‡ณIndia pirtpal_singh Ludhana

Thank you for your submission and thoughtful feedback.

Just a quick clarification on the file validator:

file_validate_extensions is the correct and Drupal-supported validator for #managed_file elements. The suggested FileExtension refers to a Symfony constraint class and is not applicable within Drupalโ€™s Form API. Using it would result in validation issues.

That said, I appreciate you raising this and will review the file upload issue on Drupal 11 in more detail to ensure full compatibility. In the next release cycle, Iโ€™ll also consider your other suggestions around multi-app ID support and batching notifications for large-scale delivery.

Thanks again for contributing โ€” looking forward to improving this together!

๐Ÿ‡ฎ๐Ÿ‡ณIndia pirtpal_singh Ludhana

Thank You for Your Feedback

I appreciate your insights and have addressed each point to ensure the Firebase UI module aligns with Drupal's best practices and maintains compatibility across versions.

โœ… src/Controller/FirebaseUiController.php

Feedback: Avoid redeclaring properties already available in ControllerBase.

Resolution: Removed explicit declarations of $currentUser and $entityTypeManager. Now utilizing inherited methods currentUser() and entityTypeManager() from ControllerBase.

โœ… src/Controller/ServiceWorkerController.php

Feedback: Avoid using \Drupal::currentUser() directly; instead, inject dependencies.

Resolution: Implemented ContainerInjectionInterface and injected the current_user service through the constructor, following Drupal's dependency injection best practices.

โœ… src/Entity/FirebaseNotification.php

Feedback: Consider using PHP attributes for plugin definitions, especially for Drupal 10.3 and above.

Resolution: Retained annotations to maintain compatibility with Drupal 8 and 9. Plan to transition to PHP attributes in the future to support Drupal 10.3 and above, ensuring broader compatibility.

โœ… src/Form/FirebaseUISettingsForm.php

Feedback: Avoid hardcoding URLs within translatable strings; use placeholders instead.

Resolution: Replaced hardcoded URLs with placeholders using the :link syntax in the t() function. This approach facilitates better localization and translation.

โœ… src/Plugin/QueueWorker/FirebaseNotificationQueueWorker.php

Feedback: Inject dependencies using the dependency injection container instead of static service calls.

Resolution: Implemented ContainerFactoryPluginInterface to inject necessary services via the constructor, aligning with Drupal's standards for service management.

These adjustments ensure that the Firebase UI module adheres to Drupal's coding standards and best practices, maintaining compatibility with Drupal 8.8 and above. Please let us know if there are any further areas that require attention or improvement.

๐Ÿ‡ฎ๐Ÿ‡ณIndia pirtpal_singh Ludhana

Thank you for the feedback.

โœ… Iโ€™ve updated the module with the following changes based on your recommendations:

  • Removed version from firebase_ui.info.yml (handled by Drupal.org packaging)
  • Changed package from 'Custom' to 'Notifications'
  • Updated the .module file docblock to: โ€œHook implementations for the Firebase UI module.โ€
  • Removed unnecessary ControllerBase inheritance where not needed, keeping only ContainerInjectionInterface

Let me know if anything else is needed.

Production build 0.71.5 2024