After updating from 7.x-2.3 to 7.x-2.4 I am no longer able to save Inclusion options for new nodes created or existing nodes on the site. I am able to save Inclusion options for the Content Type successfully. Error in the log appears to be related to Field Collection module? However it works fine when rolling back to the .3 version.
Notice: Undefined index: in FieldCollectionItemEntity->fetchHostDetails() (line 315 of \sites\all\modules\contrib\field_collection\field_collection.entity.inc).
Steps to duplicate:
Content > Add Content > Select Content Type > Scroll Down > Select XML Sitemap Section > Change Inclusion setting to 'Excluded' > Save > Edit Content Again > Inclusion remains at 'Default' setting for that Content Type.
After a rollback to 7.x-2.3 content is saving as intended.
I was able to test this and the error continues to occur in a clean drupal install.
Steps: Install latest version of Drupal 7: 7.59
Install the latest version of xmlsitemap: 7.x-2.4
after installing and enabling all modules:
content > add content > article > Title: Test Content: Test ect. > Set XML Sitemap to 'Included' > Save
Got back to re-edit the content and the XML Sitemap Inclusion continues to read 'Default'
Comment | File | Size | Author |
---|---|---|---|
#28 | xmlsitemap-2987125-27-including-tests-2989571.patch | 6.03 KB | sjerdo |
| |||
#27 | xmlsitemap-2987125-27.patch | 2.83 KB | pandaski |
Comments
Comment #2
mrgoodfellow CreditAttribution: mrgoodfellow commentedComment #3
mrgoodfellow CreditAttribution: mrgoodfellow commentedI tried this with the latest dev branch as well and I have the same issue. Tried to check the debuggin with devel but I don't see where this value is being stored.
In the DB, the xmlsitemap table is EMPTY until I use version 7.x-2.3, make changes to Inclusion option and then the page appears on that table list. When upgrading to 7.x-2.4 no changes are made to the xmlsitemap table.
Comment #4
mrgoodfellow CreditAttribution: mrgoodfellow commentedComment #5
mrgoodfellow CreditAttribution: mrgoodfellow commentedComment #6
mrgoodfellow CreditAttribution: mrgoodfellow commentedComment #7
Rory Downes CreditAttribution: Rory Downes as a volunteer commentedI have found the same issue. I took a quick look at the code but it is beyond my skills but I am happy to help test a patch.
Rory
Comment #8
rjw CreditAttribution: rjw commentedI can confirm also. Installed 7.x-2.4 on an existing site and enabled XML sitemap and XML sitemap node. Set to "Included" with priority 0.5 on a content type. Tried to set a page of that type to "Excluded." The setting for that page is not saved, it reverts to Default.
Comment #9
pandaski CreditAttribution: pandaski at govCMS (Australian Government Department of Finance) commentedThis issue is brought from the recent security release
The value is saved in the Drupal queue instead of an updating immediately. Therefore, the node cannot be updated until the CRON job.
A quick fix is saving the sitemap link initially and this link will be updated during CRON.
Comment #10
pandaski CreditAttribution: pandaski at govCMS (Australian Government Department of Finance) commentedBased on the above findings, I would like to propose an "xmlsitemap_link_presve" function in favour of the recent security release.
This function saves a sitemap link with access = 0 initially, then the Drupal CRON will update the sitemap link properly
At last, the link can be updated either after exceeding Minimum sitemap lifetime or triggering "rebuild links" through "/admin/config/search/xmlsitemap/rebuild"
Comment #11
pandaski CreditAttribution: pandaski at govCMS (Australian Government Department of Finance) commentedComment #12
mrgoodfellow CreditAttribution: mrgoodfellow commentedHi Joseph,
Thank you for your time and response. Patch #10 appears to be working successful in both my current development environment and my clean Drupal install. I will continue to test and let you know if I experience any further issues.
Comment #13
mrgoodfellow CreditAttribution: mrgoodfellow commentedI have completed testing with my team and we have confirmed this resolves the bug and have not found any other functionality issues. This patch should be ready to port to the next branch.
Comment #14
Rory Downes CreditAttribution: Rory Downes as a volunteer commentedThanks guys. That works for me.
Comment #15
tobybellwood CreditAttribution: tobybellwood at govCMS (Australian Government Department of Finance) for govCMS (Australian Government Department of Finance) commentedSetting back to RTBC as per comment in #14
Comment #16
Rory Downes CreditAttribution: Rory Downes as a volunteer commentedAh, I spoke too soon. In my last comment, I had only tested that the field could be updated. Having included two pages they do not appear in the sitemap. I ran the cron manually and when that did not work used xmlsitemap's Rebuild links function. The two pages are still not in the sitemap.
Did I do something wrong or is there a new bug?
Rory
Comment #17
pifagorSetting back to Needs review as per comment in #16
Comment #18
pandaski CreditAttribution: pandaski at govCMS (Australian Government Department of Finance) commented@Rory Downes
Can you kindly provide more configurations or steps so that I can duplicate from my local environment?
I've attached a few screenshots regarding my local environment testing
1. /admin/config/search/xmlsitemap/settings
Minimum sitemap lifetime - this is why the CRON job not working. You may want to reset it to "No minimum" for testing
2. When you rebuild the links
If "Save and restore any custom inclusion and priority links." checkbox is on can make any different results
Comment #19
pandaski CreditAttribution: pandaski at govCMS (Australian Government Department of Finance) commentedThis patch adds a variable_set that marking the sitemaps as needing regeneration.
Comment #20
pandaski CreditAttribution: pandaski at govCMS (Australian Government Department of Finance) commentedComment #21
webservant316 CreditAttribution: webservant316 commentedis this a reason to not upgrade to 2.4?
Comment #22
tobybellwood CreditAttribution: tobybellwood at govCMS (Australian Government Department of Finance) for govCMS (Australian Government Department of Finance) commentedThere have been a number of issues logged since the 2.4 release (which in itself was just a patch, not a 7.x-2.x rollup) - one of which has already been committed (https://www.drupal.org/project/xmlsitemap/issues/2986847).
You should read through them, and assess your susceptibility to them.
Comment #23
Rory Downes CreditAttribution: Rory Downes as a volunteer commented@Joseph Zhao Sorry for the delay. I had forgotten about the Minimum sitemap lifetime setting, changing that to 'No minimum' for testing worked! So Patch 10 did work.
I also tried patch 19, which I believe is intended to override this setting but if so that does not work as intended, and apears to have no impact. I am not sure however, why that is needed.
Anyway all good for me now.
Thanks for your efforts.
Rory
Comment #24
afinnarn CreditAttribution: afinnarn at University of Colorado Boulder commented@tobybellwood I only see two issues listed under the 7.x-2.4 release. Other than this issue and the one you linked to, do you have a link to any other issues caused by the recent release?
Comment #25
sjerdoSome review comments:
This could use a comment explaining why the access is set to 0.
Can these two comments be combined or could you add that the link is saved with revoked access until the node permissions are checked in the cron?
Newly added hooks should be included in the [module].api.php file for the module.
This module supports PHP 5.3+. Since short array syntaxis is included in PHP since 5.4, the long array syntaxis should be used.
Typo: xmlsitemap_link_presave
Comment #26
sjerdoComment #27
pandaski CreditAttribution: pandaski at govCMS (Australian Government Department of Finance) commentedThanks, @Rory Downes @sjerdo for the feedback
Based on the advice, the revised patch:
1. The newly added hook is included in the xmlsitemap.api.php file.
2. The long array syntaxis is used for PHP 5.3+
3. The typo is fixed.
4. Revised comments
Comment #28
sjerdoSeems good.
Adding the tests fix of #2989571: Fix tests for 7.x-2.x-dev branch to check if tests pass with this patch.
Comment #29
sjerdoRTBC for patch #27 since all tests pass.
Comment #30
saurabh-chugh CreditAttribution: saurabh-chugh as a volunteer and at TATA Consultancy Services commentedTested the patch #28 is working.
Comment #31
pifagorAdded users
Comment #33
pifagorComment #34
Kasey_MK CreditAttribution: Kasey_MK commentedAfter installing the dev branch with this commit, I agree that I can save per-node XML Sitemap settings.
However, those settings are not showing in the node's variables using Devel.
Setting those values programmatically appears to work, using...
...to exclude a node from the sitemap, for example.
Why don't those values show up on the Devel tab?
Comment #35
mrgoodfellow CreditAttribution: mrgoodfellow commentedAfter applying this patch to production and scheudle cron run, cron began to fail and multiple errors in the logs:
<code>Warning: Cannot modify header information - headers already sent by (output....
Recoverable fatal error: Argument 1 passed to xmlsitemap_node_create_link() must be an instance of stdClass, boolean given....
Cron also continue to fail.
This continued to happen until I manually rebuilt the sitemap files which created an error but triggered cron to stop running (in a loop)
It looks like the issue is related to this:
https://www.drupal.org/project/xmlsitemap/issues/2986847#comment-12722233
Where a node is both created and deleted before running cron.
I have rolled all updates back to 7.x-2.3 as 7.x-2.4 critical update does not appear to be stable at this time.
Comment #36
pifagorHello @mrgoodfellow
dev version (7.x-2.x-dev) has problems?
Comment #38
mrgoodfellow CreditAttribution: mrgoodfellow commentedHi pifagor, sorry I was referring to the 7.x-2.4 critical update release which introduced new critical issues. Wondering when this patch or the latest dev update will be pushed out as 7.x-2.5
Comment #39
darrell_ulm CreditAttribution: darrell_ulm as a volunteer commentedQuick note: I believe the opposite is now happening for version 2.6:
Included Setting Switches to Excluded for Nodes, only Default Setting can Switch to Included if that is the Default
It looks like only default and excluded settings work. If setting a node to "Included" it switches to "Excluded."