Make WordPress Core

Opened 5 years ago

Last modified 6 days ago

#48393 accepted enhancement

Fix from #38903 prevents options autoload parameter update

Reported by: anonymized_16833402's profile anonymized_16833402 Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: major Version:
Component: Options, Meta APIs Keywords: 2nd-opinion has-patch has-unit-tests
Focuses: administration Cc:

Description

This is a follow-up to #38903.

3 years ago fix for not * If the new and old values are the same, no need to update. * But this condition does not check if method call intention was to update autoload field of the option.

Currently the issue can be resolved by force update options when update_option method is called with autoload != null and check * If the new and old values are the same, no need to update. * should be skipped.

Change History (12)

#1 @desrosj
5 years ago

  • Keywords close 2nd-opinion added

Thanks for this ticket, @aboltro. And welcome to Trac!

This looks to be an intentional decision. #26394, and more specifically 26394#comment:13/[31640] shed more light. This behavior is also explicitly documented in the inline documentation.

I am going to recommend this be closed as a wontfix. But also marking for a second opinion.

Last edited 4 years ago by SergeyBiryukov (previous) (diff)

#2 follow-up: @herregroen
4 years ago

I'd disagree with the wontfix.

While it's there in the documentation it's most definitely counter-intuitive and, perhaps more importantly, there's no alternate direct function to change an options' autoload property.

This currently means that in order to change the autoload property of an option developers are forced to resort to direct $wpdb calls which shouldn't be necessary for something so simple.

I think changing update_option so that autoload is updated even if the value is unchanged would be by far the most intuitive and elegant way of doing things. Of course if neither the value nor the autoload property have been changed then nothing should happen.

#3 @joostdevalk
4 years ago

We just ran into this because @herregroen was fixing some of my code in which I was (wrongly) assuming that update_option would work to change an option's autoload value... So I disagree with the wontfix and would suggest making it so this is doable.

#4 in reply to: ↑ 2 @SergeyBiryukov
4 years ago

  • Keywords needs-patch added; close removed
  • Milestone changed from Awaiting Review to 5.8
  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

Replying to herregroen:

I think changing update_option so that autoload is updated even if the value is unchanged would be by far the most intuitive and elegant way of doing things. Of course if neither the value nor the autoload property have been changed then nothing should happen.

I tend to agree.

This ticket was mentioned in Slack in #core by chaion07. View the logs.


3 years ago

#6 @desrosj
3 years ago

  • Milestone changed from 5.8 to 5.9

This one needs more time for a proper patch and amount of testing. If it moves forwards, we should also communicate it clearly and early to developers.

I wouldn't be surprised if there are some strange edge cases where developers include the autoload in their code and it continues working only because that argument had no affect previously.

#7 @hellofromTonya
3 years ago

  • Milestone changed from 5.9 to 6.0

With 5.9 Beta 1 tomorrow and no activity on this ticket in this cycle, moving to 6.0 for further discussion and consensus.

This ticket was mentioned in Slack in #core by costdev. View the logs.


3 years ago

#9 @costdev
3 years ago

Per the discussion in the bug scrub, pinging @SergeyBiryukov to see if this ticket is still on your radar?

#10 @peterwilsoncc
3 years ago

  • Milestone changed from 6.0 to Future Release
  • Type changed from defect (bug) to enhancement

Bumping this from 6.0 as there hasn't been any activity for this cycle. It's too late to get a breaking change suitably patched, reviewed, etc before the beta.

I've changed the ticket type to enhancement as the change been discussed here is documented and was discussed in #26394.

#11 @flixos90
14 months ago

For reference, in https://core.trac.wordpress.org/ticket/58964#comment:12 @boonebgorges outlines a few caveats with the idea proposed here: If we allow changing the autoload value without updating the option, what do we do with the return value of update_option()? Should it still return false because the option was not updated, or should it return true when the autoload value was updated, despite the option not being updated? Both of these would be somewhat confusing :/

This ticket was mentioned in PR #7679 on WordPress/wordpress-develop by @debarghyabanerjee.


6 days ago
#12

  • Keywords has-patch has-unit-tests added; needs-patch removed

Trac Ticket: Core-48393

## Summary

  • This pull request introduces changes to the update_option function in WordPress, enhancing its functionality and updating the corresponding documentation.

## Code Changes

### Behavior Update:

  • If the user explicitly provides the $autoload parameter and it differs from the existing value, the autoload status will be updated even if the $value remains unchanged.
  • If both the $value and the $autoload parameters are the same as their existing values, the function will not perform an update.
  • When the $autoload parameter is set to null, it is assumed that the user has not provided a value for autoloading, and the decision to update will be based solely on the comparison of the old and new values.

### Documentation Updates

  • The documentation has been revised to reflect the new behavior of the $autoload parameter:
    • Clarified that $autoload can be updated when explicitly provided and different from the existing value, even if the $value is unchanged.
    • Enhanced explanations for better understanding of how the $autoload and $value parameters interact during the update process.

### Key Points

  • This change improves the flexibility of the update_option function, allowing for more granular control over option updates.
  • The documentation now accurately describes the behavior and implications of using the $autoload parameter.

## Additional Notes

  • These updates aim to provide clearer guidance to developers on using the update_option function and to ensure that the behavior aligns with their expectations.
Note: See TracTickets for help on using tickets.