Skip to content

Only disable Related Posts if it's possible to re-enable it#6324

Merged
gravityrail merged 4 commits intomasterfrom
fix/publicize-disabled-by-customizer
Feb 14, 2017
Merged

Only disable Related Posts if it's possible to re-enable it#6324
gravityrail merged 4 commits intomasterfrom
fix/publicize-disabled-by-customizer

Conversation

@gravityrail
Copy link
Copy Markdown
Contributor

@gravityrail gravityrail commented Feb 8, 2017 •
edited
Loading

Fixes #6323

Changes proposed in this Pull Request:

  • Only respect "enabled" flag of jetpack_relatedposts option if the ability to actually change it is enabled.

Testing instructions:

  • On a fresh install (or with freshly-deleted jetpack_relatedposts options), enable the Related Posts module
  • Go into the customizer and edit Related Posts settings (e.g. disable show date)
  • Save
  • This will trigger a bug which disables Related Posts
  • However, they should still appear since the "toggle UI" is not enabled via the jetpack_relatedposts_filter_allow_feature_toggle filter

Proposed changelog entry for your changes:

Always enables Related Posts unless disabled by filter or toggle UI is enabled.

@jeherve jeherve added [Feature] Related Posts [Pri] BLOCKER [Status] Needs Review This PR is ready for review. Bug When a feature is broken and / or not performing as intended labels Feb 8, 2017
@eliorivero
Copy link
Copy Markdown
Contributor

eliorivero commented Feb 8, 2017

I've pushed a commit that will set enabled item of jetpack_relatedposts option to true if it wasn't passed in the saving request. This solves the issue where the Customizer won't pass enabled when jetpack_relatedposts doesn't exist.

@dereksmart
Copy link
Copy Markdown
Contributor

dereksmart commented Feb 10, 2017

This seems to be working for the customizer view, as I can see related posts when customizing. But after I save, RP still acts as inactive, as in I can't see them on the front end of my site when viewing a single post.

Also when I refresh the customizer page after saving, whatever value I saved does not update properly. For example, uncheck show date, save, and on refresh the box will still be checked.

@dereksmart dereksmart added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Feb 10, 2017
@eliorivero eliorivero force-pushed the fix/publicize-disabled-by-customizer branch 3 times, most recently from 3578c73 to fd311c1 Compare February 13, 2017 15:25
@eliorivero eliorivero added [Status] Needs Review This PR is ready for review. [Status] In Progress and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. [Status] Needs Review This PR is ready for review. labels Feb 13, 2017
@eliorivero eliorivero force-pushed the fix/publicize-disabled-by-customizer branch from 0c4f408 to d5cd2a0 Compare February 13, 2017 15:43
@eliorivero
Related Posts: avoid disabling RPs when their settings are saved in C…
7bae1ca 
…ustomizer by setting enabled to true if it's not set. Also set enabled to true if one of the specific keys saved for RPs by Customizer is in the saving request.

This is because this setting doesn't have a UI in Jetpack and inadvertently not passing it while saving was resulting in it being set to false, which silently disabled Related Posts even though the module was active
@eliorivero
Tests: add tests to ensure the integrity of Related Posts options
00772f4
@eliorivero eliorivero force-pushed the fix/publicize-disabled-by-customizer branch from d5cd2a0 to 00772f4 Compare February 13, 2017 16:00
@eliorivero
Copy link
Copy Markdown
Contributor

eliorivero commented Feb 13, 2017

Thanks @dereksmart for the review.

I've updated this PR to set enabled to true if one of the Customizer options is being saved. Note that this is not a departure of the previous logic because previously to introduce RP into Customizer, if enabled was false, the other options were cleared: their value was not kept. So if user is going to save one value, it's necessarily because enabled is true. This updates solves the issue noticed by @dereksmart for users that have already run into the issue.

I've also introduced tests to ensure the integrity of the option.

@eliorivero eliorivero added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Feb 13, 2017
@gravityrail
Copy link
Copy Markdown
Contributor Author

gravityrail commented Feb 14, 2017

LGTM!

@gravityrail gravityrail added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Feb 14, 2017
@gravityrail gravityrail merged commit fd8c74a into master Feb 14, 2017
@gravityrail gravityrail deleted the fix/publicize-disabled-by-customizer branch February 14, 2017 16:58
@gravityrail gravityrail removed the [Status] Ready to Merge Go ahead, you can push that green button! label Feb 14, 2017
@jeherve jeherve added [Status] Needs Changelog [Status] Needs Testing We need to add this change to the testing call for this month's release and removed [Status] Needs Changelog [Status] Needs Testing We need to add this change to the testing call for this month's release labels Feb 14, 2017
jeherve added a commit that referenced this pull request Feb 21, 2017
@jeherve
Changelog: add #6324
8fa0954
dereksmart pushed a commit that referenced this pull request Feb 28, 2017
@jeherve @dereksmart
Jetpack 4.7: Changelog (#6331)
a9adf86 
* Changelog: update stable tag and move changelog to changelog.txt

Also remove old releases from readme.txt to keep the changelog tab short.

* Changelog: add #5883

Also update the filter's docblock to match new version.

* Changelog: add #5938

* Changelog: add #6298

* Changelog: add #3405

* Changelog: add #5941

* Changelog: add #6239

* Changelog: add #6281

* Changelog: add #6303

* Changelog: add #6018

* Changelog: add #6300

* Changelog: add #6296

* Changelog: add #6130

* Changelog: add #6292

* Readme: remove extra "on".

* Changelog: add #6307

* Changelog: add #3297

* Changelog: add #6275

* Changelog: add #6321

* Changelog: add #6297

* Readme: update the support forum link anchor.

Anchor changed when WordPress.org forums were updated to bbPress 2

* Readme: update list of a12s, it wasn't up to date anymore!

* Changelog: add #6338

* Changelog: add #6337

* Changelog: add #6335

* Changelog: add #6333

* Testing List: first version of the 4.7 testing list.

* Changelog: add #6332

* Changelog: add #6325

* Changelog: add #6326

* Changelog: add #6339

* Changelog: add #6342

* Changelog: add #6343

* Changelog: add #6346

* Changelog: add #6347

* Changelog: add #6279

* Changelog: add #6306

* Changelog: add #6312

* Changelog: add #6316

* Changelog: add #6171

* Changelog: add #6317

* Changelog: add #6246

* Changelog: add #6263

* Changelog: add #4220

* Changelog: add #5888

* Changelog: add #3406

* Changelog: add #3637

* Changelog: add #6320

* Changelog: add #5992

* Changelog: add #6322

* Changelog: add #6324

* Changelog: add #6352

* Changelog: add #6355

* Changelog: add #6360

* Changelog: add #6362

* Changelog: add #6369, #6382

* Changelog: add #6370

* Changelog: add #6375

* Changelog: add #6383

* Changelog: add #6384

* Changelog: add #6386

* Changelog: add #6395

* Changelog: add #6403

* Changelog: add #6406

* Changelog: add #6418

* Changelog: add #6419

* Changelog: add #6434

* Changelog: add #6446

* Changelog: add #6006

* Changelog: add #6096

* Changelog: add #6399

* Changelog: fix typo.

@see #6331 (comment)

* Changelog: add #6440

* Changelog: add #6443

* Changelog: add #6445

* Changelog: add #6463

* Changelog: add #6468

* Changelog: add #6471

* Changelog: add #6474

* Changelog: add #6480

* Changelog: add #6497

* Changelog: add #6499

* Changelog: add #6514

* Changelog: add #6267

* Changelog: add #5940

* Changelog: add #6492

* Changelog: add #5281

* Changelog: add #6327

* Changelog: add #6451

* Changelog: add #6525

* Changelog: add #6530
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Bug When a feature is broken and / or not performing as intended [Feature] Related Posts [Pri] BLOCKER

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gravityrail @eliorivero @dereksmart @jeherve