Revisions to the NPR API editor meta box #47

benlk · 2018-03-28T15:07:15Z

Changes

Removes the "push to NPR One" checkbox from the "Publish" meta box, and moves it to the "NPR Story API" meta box
Adds checkboxes for "Include for reading on NPR.org" and "Include for listening in NPR One" and "Set as featured story in NPR One" settings. These checkboxes include server-side and browser-side validation, such that the two "Include..." checkboxes are only checked if "Send to NPR API" is checked, and "Set as featured story in NPR One" is only checked if "Include for listening in NPR One" is checked.
- QUESTION: What's the NPRML parent ID supposed to be for sending a story to NPR.org?
- including for NPR One sets a NPRML parent ID of 319418027
- setting as featured in NPR One sets a NPRML parent ID of 500549367
Adds a field to track the date after which the story should no longer be pushed on NPR One, which defaults to 7 days from the story's publish date.
- The expiry date is stored in the post meta in ISO 8601 format, but is presented to the article editor using an HTML5 date input (or, if the useragent does not support that, the jQuery-ui datepicker) and an HTML5 time input (which falls back to text input with a placeholder)
- The expiry date is attached to the NPRML as the tag audioRunByDate with dates formatted like so: 1 Oct 2017 01:00:00 -0400, 29 Feb 2020 23:59:00 -0500
- Because WordPress does not include styles for jQuery-ui objects, this plugin of necessity includes a version of the jQuery-ui stylesheet and its spritesheet. This shim can be removed when all browsers support input type="date" .
Adds some comments and formatting fixes, generally by converting spaces to tabs
Adds a plugin text domain named nprapi

Notes

Tests will fail in PHP 5.3 because Travis-ci.org doesn't ship 5.3 on their default test-running boxes anymore. For updates to the test-running process to cover PHP 7 and 7.1, see #46

HTTPS support changes are in #44

Minor PHP 7 compat fixes are in #42

Questions:

What's the NPRML parent ID supposed to be for sending a story to NPR.org?
What version number should all this new functionality be tagged with? 1.6 was the last release of this plugin; should the docblock comments for these functions say @version 1.7?
How does including jQuery-ui's stylesheet and images from https://jqueryui.com/themeroller/ affect the plugin's license? https://jqueryui.com/themeroller/ was used to generate those; the footer of that page references https://jquery.org/license/ which says that all projects referencing it are MIT licensed. The downloaded CSS is MIT licensed according to its header comment.

For npr#26

Applies code standards and text domain.

…sheet for same

…e correctly.

…ess on saving it yet.

…datepicker. Thanks, Mozilla!

…alues in the metabox.

Co-authored by Tyler Machado <[email protected]>

…less than 10 Co-authored by Tyler Machado <[email protected]>

…one_featured, set those parent IDs on the NPRML array Outstanding question: What's the parent ID for "Include for reading on NPR.org"?

… for meta-boxes.php

jgarlow · 2018-04-12T20:10:15Z

QUESTION: What's the NPRML parent ID supposed to be for sending a story to NPR.org?

Actually the NPR.org code is taking a completely different approach - there will be a new separate API which can be used to mark a story available for syndication. This isn't yet ready to be used at this time.

jgarlow · 2018-04-12T20:23:51Z

Note that the SOW just says to replace the current functionality that says "Push to NPR" to say "Send to the NPR API" - you don't have to make any other changes there. This does not require you to say anything about NPR.org.

jgarlow · 2018-04-12T20:37:29Z

meta-boxes.php

+ * @todo When there is better browser support for input type="datetime-local", replace the jQuery UI and weird forms with the html5 solution. https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/datetime-local
+ */
+function nprstory_publish_meta_box( $post ) {
+	$helper_text = __( 'Push this story to NPR:', 'nprapi' );


I think this is supposed to be "Send to NPR API"

In the current version of the plugin, $helper_text is displayed atop the meta box, but it's not used at all in this pull request, so I'll remove it.

Here's how it was formerly used:

nprapi-wordpress/meta-boxes.php

Line 14 in 6cdfea4

<p class="helper-text"><?php echo $helper_text; ?></p>

And here's where it isn't being used at all, now:

jgarlow · 2018-04-12T20:37:52Z

meta-boxes.php

+	$attrs = array( 'id' => 'ds-npr-update-push' );
+
+	if ( $is_disabled ) {
+		$helper_text = __( 'Publish this story in order to push it to NPR.', 'nprapi' );


Send to NPR API

jgarlow · 2018-04-12T20:40:49Z

meta-boxes.php

+			printf(
+				'<label><input value="1" type="checkbox" name="send_to_api" id="send_to_api" %2$s/> %1$s</label>',
+				__( 'Send to NPR API', 'nprapi' ),
+				checked( $nprapi, '1', false )


I don't know what the function checked() does - why are there two parameters a '1' and a false? - so just confirm that this is setting it to checked by default per the SOW.

checked() is a WordPress built-in function that in this case outputs the HTML checked="checked" if the value in $nprapi is equal to the default value, which is the string '1'. The false parameter here just means that the function's output should be returned instead of echoed; this is done because the output of checked() is being used as a parameter on printf() for inclusion in the string '<label><input value="1" type="checkbox" name="send_to_api" id="send_to_api" %2$s/> %1$s</label>' at the point %2$s.

The default value of $nprapi is set a few lines up from here, at this line:

$nprapi = get_post_meta( $post->ID, '_send_to_nprone', true ); // 0 or 1 if ( '0' !== $nprapi && '1' !== $nprapi ) { $nprapi = '1'; } // defaults to checked; unset on new posts

get_post_meta() doesn't provide a way to have a default value. When called in the post editor for a post that doesn't exist yet, or for a post that doesn't have this meta key saved, get_post_meta() will return false. Because the boolean false is not type-strictly equal to the strings '0' or '1', the conditional here sets $nprapi to '1'.

If get_post_meta() returns a value that is '0' or '1', then we use that value.

checked() says the checkbox is checked if $nprapi is '1', so the checkbox defaults to checked because $nprapi defaults to a value that means the checkbox should be checked.

In the event of something weird being saved as this post meta, $nprapi would still be set to '1'. Saving the post calls the function nprstory_save_send_to_api() which would overwrite the weird value, or an unset value, with the value of the checkbox. If the user hasn't unchecked the box, then the saved value will be that the box should be checked.

(Sorry for really long explanation; I rubber-ducked here and found a bug as a result.)

Thank you for the explanation!

jgarlow · 2018-04-12T20:41:14Z

meta-boxes.php

+				'<li><label><input value="1" type="checkbox" name="send_to_org" id="send_to_org" %2$s/> %1$s</label></li>',
+				__( 'Include for reading on NPR.org', 'nprapi' ),
+				checked( get_post_meta( $post->ID, '_send_to_org', true ), '1', false )
+			);


Remove this section for now - we are not ready for it.

Removed in 493ef78.

jgarlow · 2018-04-12T20:43:28Z

push_story.php

+ * @param Int $post_ID The post ID of the post we're saving
+ * @since 1.7
+ */
+function nprstory_save_send_to_org( $post_ID ) {


Again remove this

Removed in 493ef78

jgarlow · 2018-04-12T20:44:50Z

I don't know enough about this code to do a full review of it but I left some comments that I hope are helpful

benlk · 2018-04-17T00:09:36Z

Yes, those were helpful comments. Does my response to the checked() question make sense?

benlk · 2018-04-20T18:28:41Z

Merging with approval from Olubunmi Odumade via email.

This fixes the issue described in #57, where the URL parameter ds_npr_update_push was removed along with the 'Push to NPR' submit button, but the URL parameter triggering the nprstory_api_push function was not changed.

benlk added 30 commits March 19, 2018 12:40

Add text domain to plugin readme.txt

b1c8228

For npr#26

Clean up meta-boxes.php

654bae0

Applies code standards and text domain.

Add useless check boxes to the NPR Story API metabox, enqueue a style…

f855752

…sheet for same

Create the checkboxes in the NPR Story API box and make sure they sav…

4bd9606

…e correctly.

default the 'send to npr api' box to checked

2eaf1c6

Checkbox validation javascript

099130b

Better recursion, cleaner form validation javascript

d9a7740

Metabox validation is less DRY than form javascript.

2896c9b

console.log

445b8e2

Date picker boilerplate elements

dc50455

Improvements to the presentation of the datepicker form, but no progr…

06f7623

…ess on saving it yet.

enqueue jquery-ui-datepicker

c8d5212

Add the jquery-ui datepicker

776d6f1

Nah, use the datepicker only if there's not a native solution to the …

3e900c2

…datepicker. Thanks, Mozilla!

Add the fallback assets for the jquery ui datepicker

d1d4329

Fix recursion upon initilization in the metabox javascript validation

727fce9

Add file contents to some PHP files that were missing them

a7a546a

Save the NPR One expiry datetime as post meta, and output the saved v…

5ce08d9

…alues in the metabox.

Notes on what the next steps are for the datepicker box

5ad0a49

Minor cleanup in nprstory_api_push

7ab8c3a

import @tylermachado's datepicker functionality

93a6126

Co-authored by Tyler Machado <[email protected]>

fix off-by-one error on day number, and left pad the minutes if it's …

e730ef6

…less than 10 Co-authored by Tyler Machado <[email protected]>

sanitize this one weird error field

f85dc68

Formatting and internal commenting in nprstory_post_to_nprml_story

39c9603

Based on the post meta values of _send_to_org, _send_to_one, and _npr…

a6b989c

…one_featured, set those parent IDs on the NPRML array Outstanding question: What's the parent ID for "Include for reading on NPR.org"?

another spacing fix

b77cd0c

Historical note

b8a7b81

Add the audioRunByDat field to the pushed NPRML

4277fe5

Remove a test for a function that no longer exists; create tests file…

7155975

… for meta-boxes.php

actually remove that missing test

8ce7dc4

jgarlow reviewed Apr 12, 2018

View reviewed changes

benlk added 4 commits April 16, 2018 19:21

Merge branch 'master' into 9-expiration-date-picker

1210131

remove $helper_text, as it is no longer used

5f47778

set nprapi to string 1, not int 1

4234bd6

remove send_to_org checkbox, save function, API push function

493ef78

Merge branch 'master' into 9-expiration-date-picker

22af682

benlk merged commit 932b10e into npr:master Apr 20, 2018

This was referenced Apr 20, 2018

Make all strings translatable, generate translation files. #26

Open

Release 1.7 #49

Merged

Add an expiration date picker in metabox INN/WP-DS-NPR-API#9

Closed

benlk mentioned this pull request Sep 26, 2018

Problems pushing content to the API #57

Closed

benlk mentioned this pull request Sep 26, 2018

Resolve problems pushing to and pulling from Story API #60

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revisions to the NPR API editor meta box #47

Revisions to the NPR API editor meta box #47

benlk commented Mar 28, 2018 •

edited

Loading

jgarlow commented Apr 12, 2018

jgarlow commented Apr 12, 2018

jgarlow Apr 12, 2018

benlk Apr 16, 2018

jgarlow Apr 12, 2018

jgarlow Apr 12, 2018

benlk Apr 17, 2018

jgarlow Apr 17, 2018

jgarlow Apr 12, 2018

benlk Apr 17, 2018

jgarlow Apr 12, 2018

benlk Apr 17, 2018

jgarlow commented Apr 12, 2018

benlk commented Apr 17, 2018

benlk commented Apr 20, 2018

Revisions to the NPR API editor meta box #47

Revisions to the NPR API editor meta box #47

Conversation

benlk commented Mar 28, 2018 • edited Loading

Changes

Notes

Questions:

jgarlow commented Apr 12, 2018

jgarlow commented Apr 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgarlow commented Apr 12, 2018

benlk commented Apr 17, 2018

benlk commented Apr 20, 2018

benlk commented Mar 28, 2018 •

edited

Loading