Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metadata improvements (fixes #804) #832

Merged
merged 31 commits into from Jul 5, 2017

Conversation

@greatislander
Copy link
Collaborator

commented Jul 5, 2017

This PR:

  • Improves metadata API endpoint, adding audience, ISBN, and full BISAC subject support;
  • Refactors metadata class to implement JsonSerializable, moving static functions to a namespace;
  • Hides a bunch of book info fields that may not be needed for ordinary users (fixes #804);
  • Adds an option to show expanded book information for advanced users;
  • Fixes book license display via metadata endpoint;
  • Adds support for multiple editors and translators;
  • Adds sensible fallbacks for pb_copyright_date and pb_author_file_as.
greatislander and others added 22 commits Jul 4, 2017

@greatislander greatislander added this to the 4.0 milestone Jul 5, 2017

@greatislander greatislander self-assigned this Jul 5, 2017

@greatislander greatislander requested a review from connerbw Jul 5, 2017

@codecov

This comment has been minimized.

Copy link

commented Jul 5, 2017

Codecov Report

Merging #832 into dev will increase coverage by 1.02%.
The diff coverage is 77.15%.

@@             Coverage Diff             @@
##               dev     #832      +/-   ##
===========================================
+ Coverage     30.7%   31.72%   +1.02%     
+ Complexity    2732     2720      -12     
===========================================
  Files           77       78       +1     
  Lines        14122    14260     +138     
===========================================
+ Hits          4336     4524     +188     
+ Misses        9786     9736      -50
@@ -45,6 +47,10 @@
echo '<dc:creator opf:role="aut"';
if ( ! empty( $meta['pb_author_file_as'] ) ) {
echo ' opf:file-as="' . $meta['pb_author_file_as'] . '"';
} else {
$nameparser = new Parser();
$author = $nameparser->parse( $meta['pb_author'] );

This comment has been minimized.

Copy link
@connerbw

connerbw Jul 5, 2017

Member

This library can throw exceptions. What happens if it's a bad name? Try/Catch?

@@ -69,9 +71,11 @@
if ( ! empty( $meta['pb_author_file_as'] ) ) {
echo $meta['pb_author_file_as'];
} elseif ( ! empty( $meta['pb_author'] ) ) {
echo $meta['pb_author'];
$nameparser = new Parser();
$author = $nameparser->parse( $meta['pb_author'] );

This comment has been minimized.

Copy link
@connerbw

connerbw Jul 5, 2017

Member

This library can throw exceptions. What happens if it's a bad name? Try/Catch?

@@ -29,8 +29,6 @@ public function test_update_font_stacks() {
$this->assertArrayHasKey( 'part', $wp_meta_boxes );
$this->assertArrayHasKey( 'metadata', $c->metadata );
$this->assertArrayHasKey( 'general-book-information', $c->metadata['metadata'] );
$this->assertArrayHasKey( 'additional-catalogue-information', $c->metadata['metadata'] );

This comment has been minimized.

Copy link
@connerbw

connerbw Jul 5, 2017

Member

Instead of removing this test, can we add another test that checks if its hidden? (Do update_option( 'pressbooks_show_expanded_metadata' 1); and test if additional-catalogue-information is there.)

This comment has been minimized.

Copy link
@greatislander

greatislander Jul 5, 2017

Author Collaborator

@connerbw All of these requests have been addressed—want to take one last look before I merge?

This comment has been minimized.

Copy link
@connerbw

connerbw Jul 5, 2017

Member

Done.

<div id="expanded-metadata-panel" class="postbox">
<div class="inside">
<p><?php _e( 'The book information you enter here appears on your book’s cover and title pages and in the metadata of your webbook and exported files.', 'pressbooks' ); ?></p>
<?php if ( ! $show_expanded_metadata && $show_expanded_metadata !== null ) { ?>

This comment has been minimized.

Copy link
@connerbw

connerbw Jul 5, 2017

Member

$show_expanded_metadata = show_expanded_metadata(); never returns null.

/**
* Should we show expanded metadata fields or not?
*
* @return bool | null

This comment has been minimized.

Copy link
@connerbw

connerbw Jul 5, 2017

Member

Function does not return null.

@greatislander greatislander requested a review from connerbw Jul 5, 2017

try {
$author = $nameparser->parse( $meta['pb_author'] );
$author_file_as = $author->getLastName() . ', ' . $author->getFirstName();
} catch ( Exception $e ) {

This comment has been minimized.

Copy link
@connerbw

connerbw Jul 5, 2017

Member

Should be \Exception. Optionally, my preference would be to catch \HumanNameParser\Exception\NameParsingException instead of searching the string for the error.

try {
$author = $nameparser->parse( $meta['pb_author'] );
echo $author->getLastName() . ', ' . $author->getFirstName();
} catch ( Exception $e ) {

This comment has been minimized.

Copy link
@connerbw

connerbw Jul 5, 2017

Member

Should be \Exception. Optionally, my preference would be to catch \HumanNameParser\Exception\NameParsingException instead of searching the string for the error.

@greatislander greatislander merged commit e9ba55b into dev Jul 5, 2017

3 checks passed

codecov/patch 77.15% of diff hit (target 30.7%)
Details
codecov/project 31.72% (+1.02%) compared to e0b339b
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@greatislander greatislander deleted the metadata-improvements branch Jul 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.