Problem/Motivation

I have 2 search engines defined in my site, and everytime I export my site configuration they are also exported.
The reason for it is because it is saving the last_submitted date at config file.
I think we should not let that export because this value doesn't look like a site configuration and makes developers always wonder if they changed something at search engines or not.

Steps to reproduce:

  1. Create a search engine at /admin/config/search/simplesitemap/engines
  2. Export site configuration
  3. Run Cron (this action should submit the search engines)
  4. Export site configuration again

Proposed resolution

Remove last_submitted flag from search engines.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Diego_Mow created an issue. See original summary.

gbyte’s picture

Title: Exporting Configuration about Search Engines add last submitted flag » Move search engine last submitted timestamp away from configuration
Version: 8.x-3.5 » 8.x-3.x-dev
Category: Bug report » Task

Not a bug, but the request makes sense - will look into it.

PCate’s picture

The State API is designed to store values such as these so they are not part of configuration.

gbyte’s picture

Sure it is; I did not catch the problem when reviewing this submodule. This will require some data model changes though as right now the timestamp is saved as part of the entity data. Feel free to submit a patch to accelerate the process.

Spokje’s picture

Patch attached that takes the quick (not too dirty?) road by _not_ exporting the `last_updated` field and using the already present State to fill the last_update field in the simple_sitemap_engine Entity

Spokje’s picture

Status: Active » Needs review
Diego_Mow’s picture

Status: Needs review » Reviewed & tested by the community

Worked fine for me!

gbyte’s picture

Status: Reviewed & tested by the community » Needs review
gbyte’s picture

Status: Needs review » Needs work

@Spokje

Thanks for the patch! The status table however will now show the global 'last submitted' date instead of the date the specific search engine was submitted and we have an unnecessary last_submitted field in the entity. I recon we should move the last_submitted data for each engine to state instead.

lolcode’s picture

Status: Needs work » Needs review
FileSize
11.88 KB

I have attempted to move this forward. There are now individual state variables for each engine.

gbyte’s picture

@lolcode Thank you, looks good, will be doing some testing soon and I will get back to you.

AndyF’s picture

Thanks @lolcode! Would be great to see this land; I'm really tight on time to actually test it out but I gave it a quick once-over, makes sense to me 👍

  1. +++ b/modules/simple_sitemap_engines/simple_sitemap_engines.install
    @@ -10,4 +10,24 @@
    +function simple_sitemap_engines_update_8001() {
    

    I think this should be 8101.

  2. +++ b/modules/simple_sitemap_engines/simple_sitemap_engines.install
    @@ -10,4 +10,24 @@
    +    $config = \Drupal::configFactory()
    +      ->getEditable("simple_sitemap_engines.simple_sitemap_engine.$engine_id");
    +    $config->clear('last_submitted')->save();
    
    +++ b/modules/simple_sitemap_engines/src/Plugin/QueueWorker/SitemapSubmitter.php
    @@ -2,11 +2,13 @@
     
    

    Is it worth setting the state variable before clearing the timestamp on the config entity?

  3. Not critical but there are some places where comments or type declarations use a class rather than the interface, eg. State instead of StateInterface

Thanks again!

lolcode’s picture

Regarding your comments:

1. I think it should be 8301. Thanks I had copied from the older patch.
2. OK. Done.
3. Fixed.

Attaching an updated patch.

  • gbyte committed ba8d0c0 on 8.x-3.x authored by lolcode
    Issue #3118519 by lolcode, Spokje, gbyte, Diego_Mow, AndyF: Move search...
gbyte’s picture

Status: Needs review » Fixed

Thanks for your input and patience guys, this is now commited.

@lolcode I cleaned up a bit and removed the injected $configFactory in \Drupal\simple_sitemap_engines\Plugin\QueueWorker\SitemapSubmitter and a few variables, as that bit of code was not used anywhere. I still put you down as author, so feel free to review and object.

Status: Fixed » Closed (fixed)

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