Contrib.social

Allow editing of "Notification interval" options

Open on Drupal.org →
Created on 23 March 2018, over 6 years ago
Updated 1 March 2024, 6 months ago

Currently, we're using a list_integer base field with fixed options for the "Notification interval" of saved searches.
While that gives us most of the functionality we need out-of-the-box, it doesn't seem possible this way to let the admin configure the set of options (or the default), or set it to a fixed interval. (It's possible, via the field UI, to hide the field from users, but it's apparently not possible to change the default value, so all saved searches would be stuck at "Never".)

This definitely has to be amended, but I'm not familiar enough with the Field API yet to know how to best resolve this. (This is my first "actual" content entity type, I think.)

We can override the base fields per-bundle, but that won't make them configurable either – we'd have to provide the whole form UI for changing the default and options ourselves, like in D7. Which would be a possible solution, but a bit disappointing since configurable list_integer fields have already the exact functionality we need (although I guess the "Fixed interval" solution would be a bit makeshift in this case).

There are also "base field override" config entities, which users can create themselves to override existing base fields. Since I think they don't have a UI yet (i.e., you'd have to manually write the config and import it), this is not really proper UX. Also, it seems you can't override all the settings that way. (I was able to change the default value, but not the options, this way. But maybe I did something wrong.)

Finally, we could remove the base field completely and just provide a field storage config and field instance configs per default for all saved search types. I guess that would work pretty well in general, but then we couldn't rely on the field being present anymore – we'd have to disable notifications when the field is removed for a certain type. (Or switch to some default behavior.) Or can we add a dependency on that field instance config from the type? (Since that would be circular, I guess not. Also, we may not want to.)

Anyways, input from people more familiar with the Field API would be very much welcome!

📌 Task
Status

Fixed

Version

1.0

Component

Code

Created by

🇦🇹Austria drunken monkey Vienna, Austria

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment 10 months ago →
    🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

    Is this really a beta blocker?

  • Issue was unassigned.
  • Status changed to Needs review 10 months ago
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & SQLite 3.27
    last update 10 months ago
    32 pass, 3 fail
  • Comment 10 months ago →
    🇦🇹Austria drunken monkey Vienna, Austria

    Finally got around to working on this again. As said above, I’m going forward with approach number 1 – patch attached. Unfortunately, there is still a test fail which I can’t quite figure out, and which I also can’t reproduce testing manually, so that’s a bit annoying, but I thought maybe someone still wants to review already, or maybe even has an idea how to fix the test.

    I also implemented dorficus’ suggestion of putting the different search type form sections into vertical tabs, which really looks qute good. I was almost done with it when I realized it was completely out of scope here, so to not let that go to waste I still included it.

  • Comment 10 months ago →
    System Message

    The last submitted patch, 15: 2955515-15--configurable_notification_interval.patch, failed testing. View results →
    - codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.4 & SQLite 3.27
    last update 9 months ago
    48 pass
  • Comment 9 months ago →
    🇦🇹Austria drunken monkey Vienna, Austria

    OK, I think I figured it out.
    Would be great to get some feedback/tests/reviews on this and then we can finally have a Beta release.

  • Comment 9 months ago →
    System Message
  • Status changed to Fixed 9 months ago
  • Comment 9 months ago →
    🇦🇹Austria drunken monkey Vienna, Austria

    Alright, would have been better with feedback, but I’m just rolling with it now to finally get this resolved.
    Merged. Thanks again, everyone!

  • Comment 8 months ago →
    System Message

    Automatically closed - issue fixed for 2 weeks with no activity.

  • Comment 6 months ago →
    lelkneralfaro

    I haven't looked into the code enough to figure out what might be going on and suggest a solution so I just wanted to report an issue/observation based on my use of the module.

    Background:
    I am currently using search_api_saved_searches version 8.x-1.0-rc1

    I have been using the 'Default' search type created from installation of the module over a year ago.

    Observation:

    The notification interval field used to be available on the 'Default' search type but no longer is. When I create a new search type, 'notification interval' is available as a form field.

    Suggested solution (to take with a grain with salt):

    Maybe an update hook is required to update configuration for already created search types.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024