• Resolved ranchhand6

    (@ranchhand6)


    I am interested in finding out where you’re going to go with the JSON API for this plugin.

    Context: I have a need to replace a printed organization roster with a simple iPhone/Android app. We’ve been running two databases for the organization, one in the Connections plugin to provide an on-line listing, and one to print the roster. Due to _strong_ views within the organization, we need to replace the printed version with an off-line equivalent. Our current thinking is to use the Connections roster as the central data store, then query using a JSON GET to update local stores in the phone apps.

    I have reviewed your alpha code in 8.5.26 and it looks promising. I have a couple of questions:

    1. It seems like you are using the WP JSON API V1 code as a base rather than the WP JSON API V2 code which is currently in plugin form, but supposed to be integrated into the core in December of this year. Is there a reason that you chose V1 over V2?

    2. Where is this on your priority list? Are you actively working this code or does your focus lie elsewhere at the moment?

    3. I looked for a git repository for this plugin with the thought of possibly forking and contributing via a pull request, but it didn’t look like you use that workflow. Do you have a preferred method of accepting contributed code if that’s what my organization decides to do?

    Thanks!

    AWD

Viewing 15 replies - 1 through 15 (of 15 total)
  • Plugin Author Steven

    (@shazahm1hotmailcom)

    @

    re: t seems like you are using the WP JSON API V1 code as a base rather than the WP JSON API V2 code which is currently in plugin form, but supposed to be integrated into the core in December of this year. Is there a reason that you chose V1 over V2?

    Nope, it is V2 b15. Included the same way WooCommerce does. After 4.7 ships, the library will be removed.

    re: Where is this on your priority list? Are you actively working this code or does your focus lie elsewhere at the moment?

    I would say about third on the list after support/maintenance of the commercial add-ons and custom dev for Connections based solutions for third parties. So, pretty high priority. I have plans on using the API to improve Connections itself. I’ve been wanting a REST API for quite awhile, but I wanted to base it on/extend the core WP REST API … they just took a lot longer than I thought/hoped to get it merged.

    re; I looked for a git repository for this plugin with the thought of possibly forking and contributing via a pull request, but it didn’t look like you use that workflow. Do you have a preferred method of accepting contributed code if that’s what my organization decides to do?

    Well, I don’t use it only because I have not received very many pull request. The few I have, code needed cleanup up to be more in WP coding standards or the implementation of a feature was going to come in conflict with plan I already had. So, those pull requests I simply ended up manually merging.

    So… I would actually be very, very, open to pull requests and would greatly welcome them. It is actually why I put a link to the Github repo on the plugin description page.

    re: I am interested in finding out where you’re going to go with the JSON API for this plugin.

    The categories part of the API is beta/complete. I was able to base most of the code off the terms code in core WP REST API. After 4.7 ships, I’ll bring the code back in alignment and say it is complete.

    Now, the actual entry portion of the API is definitely in the experimental stages. I basically need to create my own schema since basing it on pages/post is not the right way forward. I basing the schema off this:

    http://json-schema.org/card

    One that is complete, I will also have a GeoJSON endpoint to allow data from Connections to be fed directly into mapping API such as Google Maps. Well, actually it’ll be a variant of a schema still in the proposal stages, GeoJSON-LD. Primary reasoning is so the entries details can be passed and not have to be requested via a second request. Actually, now that I think of it, this will be developed in tandem with a endpoint based on the JSON-LD schema so data can be feed into a variety of other API which can consume the data. The trouble I am having with the JSON-LS is that is is built upon Schema.org and does not seem to descriptive enough. So I spend a lot of time researching which field in Connections should be match to which property in Schema.org.

    Sooo… all this said, If you and your company decides to base your solution on Connections and its forthcoming REST API, I will gladly review and accept pull requests.

    Hope I have answered your questions thoroughly!

    Thread Starter ranchhand6

    (@ranchhand6)

    Thanks for your response, this is very helpful.

    One of the things that confused me was your use of the v1 namespace in the routing. I had assumed from other code that I’d looked at that this distinguished the v1 from the v2 WP API library, but reading further in the documentation, I see that it’s an internal API version indicator. We’re learning all the time.

    I also missed your github reference in the Description, but I have it now. I’ll talk it over with our customer here and see how they want to proceed.

    Again, thanks for your help.

    AWD

    Thread Starter ranchhand6

    (@ranchhand6)

    Not sure if this is the right place to ask a programming question, if there’s a different way to communicate, I’m more than happy to use whatever means you are comfortable with. Also, I haven’t been doing PHP for awhile now, so apologies is this error resulted from simple oversight.

    We decided to go ahead and put a little work into the REST API interface, so I’ve forked your github repo and have been working my way forward using WP_Unit tests. Please consider everything done to date as draft, we are not married to any of it.

    I have a question about an interaction I’m seeing between class.entry-data and class.actions. We’re mocking some data into an entry, and the thing we’re trying to do is to attach an image in the WP_Unit test so that we can test and debug the output of the REST API. The problems seems to be that for everything _but_ the image, the mocked data seems to work fine but with the image data the “update” action fails. While exploring this, we noticed the following:

    At class.entry-data->update():5003 (or so), after submitting the CN_ENTRY_TABLE update, you use the following check before allowing the remainder of the tables to update:

    
           if ( FALSE !== $result ) {
    

    In our test case, this succeeds because the rest of the updates work. The variable $result is returned later in the method as the result of the entire method.

    However, at class.entry-actions->process():789 (or so), you use the following check on the return:

    
    	if ( $entry->update() == FALSE ) {
    

    In our case, this fails.

    Indeed, when we put some testing code into the method,

        
        echo "After cn_update-entry ". ((FALSE !== $result) ? "true" : "false" ). " \r\n";
        echo "After cn_update-entry ". ((FALSE == $result) ? "FALSE" : "TRUE" ). " \r\n";
    

    … we get these results:

    After cn_update-entry true
    After cn_update-entry FALSE

    This seems a little counter-intuitive.

    I hesitate to go much deeper into the base code without consulting with you. I have a branch and a test that stimulates the issue, if you care to look at it. Again, what we’re trying to do is to attach an image to a test entry in order to test the output, if you have an easy way to do that without looping through the class.entry-actions->add() and class.entry-actions->update() we’re more than happy to take a different approach.

    Thanks.

    AWD

    • This reply was modified 7 years, 6 months ago by ranchhand6.
    Plugin Author Steven

    (@shazahm1hotmailcom)

    @ranchhand6

    re: Not sure if this is the right place to ask a programming question, if there’s a different way to communicate, I’m more than happy to use whatever means you are comfortable with

    Github probable would be best. I believe if you create an issue and tag me, @shazahm1, I’ll get notified and will respond.

    So, if I read this correctly, class.entry-actions->process():789 always returns FALSE? It should always be FALSE or 1 since $result is just the return value of $wpdb->update() This method returns FALSE on failure and on success the number of rows affected by the update query which should always be 1 in this case.

    In your debug code add this:

    echo "After cn_update-entry " . ( json_encode( $result, 128 ) ) . " \r\n";

    What is the result?

    Thread Starter ranchhand6

    (@ranchhand6)

    Thanks for the explanation. I’m getting a ‘0’ for some reason.

    
    After cn_update-entry 0
    

    Hmm. I know the record exists because I can see it during a previous portion of the test. It makes me suspect that somehow I’m not doing the update() correctly.

    Let me work with it some more. If I have other questions, I’ll open an issue on github.

    Cheers!

    AWD

    Plugin Author Steven

    (@shazahm1hotmailcom)

    @ranchhand6

    Hmmm… 0… that seems to indicate that $wpdb->update() was successfull but no rows were affected.

    Verify that the id is correct (must match the id in the database) before issuing the $entry->update() method.

    Thread Starter ranchhand6

    (@ranchhand6)

    I’m using the id from the add immediately prior, looking at the $response object and getting the id from there and putting it into the update.

    However, not seeing the entry in the database after the test. I suspect this is because the test framework is using transactions to isolate the test data, but am not deeply familiar enough with WP_Unit as yet to know if this is true. Is there a caching layer I need to flush after the add?

    Plugin Author Steven

    (@shazahm1hotmailcom)

    @ranchhand6

    Unfortunately I am not familiar with WP_Unit either, so I do not really know what you’ll need to do to ensure the test insert works correctly.

    I am working on the REST API code myself and it is becoming very obvious that I am going to have to better refine the code so it is much easier to insert a new entry, attach images and assign categories. Presently all the existing methods assumes action being taken via input from the admin form submissions.

    I’m thinking of adding a new set of functions such as cn_insert_entry(), cn_attach_image(), cn_assign_term() and so on to create a much more well defined API which aligns to the how one would programmatically insert WP posts and such to provide a familiar “feel” when using them. The current methods would be refactored to use these “core” API functions.

    Seems like a good time to learn about and add to my workflow PHPUnit and Travis CI. I already use Scrutinizer to help reduce bugs and other coding errors

    Thread Starter ranchhand6

    (@ranchhand6)

    I support your idea of the new functions, I had been thinking along the same lines but wanted to make sure the testing framework was working before launching off into that.

    I’ve resolved my problem down into a failure to retrieve the record during the update. It’s puzzling to me that the add works in the test context but the update does not. I thought that perhaps I was on to something when I realized that I was passing a string rather than a integer into the update method, but then I realized that you had protected against that with an absint() call in the $entry->set() method in class.entry-actions.php.

    Sigh. Still looking.

    Thread Starter ranchhand6

    (@ranchhand6)

    I finally resolved it down to a transaction isolation problem between tests in the same file. I was adding a mocked entry in one test and was able to retrieve it in another so I assumed that I would be able to update it as well. This is not the case. The test framework apparently prevents this. When I added the mocked entry inside the test case I was trying to do the update in, everything started working as expected.

    Write it up to my inexperience with WP_Unit.

    Plugin Author Steven

    (@shazahm1hotmailcom)

    @ranchhand6

    Great to hear you got that figured out! That is great little tidbit to keep in mind.

    I spent the day “wiring” up Travis CI and I just finally got it build the test matrix without failing.

    https://travis-ci.org/Connections-Business-Directory/Connections

    It only has one unit test of rather dubious value simply to prove that it works. It is a good start for now and I’m sure it’ll be of great value as I write these new functions.

    Since it seems you may have a bit more experience with unit testing. I have a question for you.

    Here’s my one test:

    https://github.com/Connections-Business-Directory/Connections/blob/develop/tests/category/tests-Uncategorized_Exists.php

    How would you deal with a test where a result could be a WP_Error class instance? I do this a lot in my code for error detection. I’d like to be able to catch in in the tests too vs have PHP warnings and notices flooding the console.

    Thread Starter ranchhand6

    (@ranchhand6)

    I can’t say I’m an expert, but the approach you took looked reasonable to me. I think I would have done two separate assertions instead of putting the test in an if statement because I think it reads better. Something like:

    
    public function test_category_exists_alternate() {
    
      $term = cnTerm::getBy( 'name', 'Uncategorized', 'category' );
    
      $this->assertFalse( is_wp_error( $term ) , 'WP Error occurred. Uncategorized category not found.');
    
      $this->assertEquals(
        array(
          'name' => 'Uncategorized',
          'slug' => 'uncategorized',
        ),
        array(
          'name' => $term->name,
          'slug' => $term->slug,
        ),
        'Uncategorized category not found.'
      );
    }
    

    Same thing, just a bit different stylistically.

    • This reply was modified 7 years, 6 months ago by ranchhand6.
    Plugin Author Steven

    (@shazahm1hotmailcom)

    @ranchhand6

    Ahhh, yes, that actually does make much more sense. And then all I need to do is ensure the object properties exists be adding the to the array to compare. That way I will not get PHP warnings about access properties of non object in the console.

    I like that better, thanks!

    Thread Starter ranchhand6

    (@ranchhand6)

    My pleasure!

    I synchronized my fork to your /develop branch, so I see where you are. With regards to the API, what can I do to help that keeps me out of your way?

    Plugin Author Steven

    (@shazahm1hotmailcom)

    @ranchhand6

    Hmmm… good question. Working on the exact same code only makes things more difficult for you and I do not want that 🙂

    I guess the first thing I can do to help you is to let you know exactly what I’m working on and what I plan on next.

    REST API Response: I’m working on finishing the response to get entries and a single entry. This response will include only data from the main connections table from the db and the address data. The phone, email and stuff will come after. Doing just the addresses is enough for the moment so I can flush out and determine how the JSON response will be structured.

    After I am happy with that, I’ll define the schema response.

    Then I plan on focusing on adding a new entry via the REST API. And here is where is gets more difficult because I’ll need to work out how I want to do the helper functions to make it easier to programmatically insert/edit/edit entries as I mentioned this reply. One other item that I have not mentioned yet which makes this a little more difficult is I also want to make sure these helper functions work with wp-cli too.

    If you have some ideas on how these helper functions should be designed. I would like to hear them. That would be helpful. I’m certain passing all data as one large array as it is now makes it pretty difficult because the array structure is deep and specific… works well enough when being generated by a form though. At this point I’m envision something like this:

    
    $entry = new cnEntry;
    $entry->setName( $data );
    
    // The addAddress would be new and would be some some sort of collection class.
    // This would simplify adding individual addresses since the 
    // current method relies on a nested array.
    $entry->addAddress( $data-1 );
    $entry->addAddress( $data-2 );
    
    // Or maybe this instead.
    // More verbose but the code would be abstracted out into
    // separate classes and methods making it far more maintainable and unit testable.
    $addressCollection = new cnAddresses;
    $address_1 = new cnAddress;
    $address_1->addLineOne( $data );
    $address_1->addCity( $data );
    $address_1->addState( $data );
    $address_1->addCity( $data );
    
    $address_2 = new cnAddress;
    $address_2->addLineOne( $data );
    $address_2->addCity( $data );
    $address_2->addState( $data );
    $address_2->addCity( $data );
    
    $addressCollection->addAddress( $address_1 );
    $addressCollection->addAddress( $address_2 );
    
    $entry->addAddresses( $addressCollection );
    
    $entry->save()
    

    Now, this does not seem like it will work cn_insert_entry() function, though the above could be the logic within the function. But how would the data be passed? As mentioned, passing one large multiple dimensional array is too difficult (pretty much the exact way it is now).

    I opened an issue on GitHub. Let move the discussion there.

    https://github.com/Connections-Business-Directory/Connections/issues/468

Viewing 15 replies - 1 through 15 (of 15 total)
  • The topic ‘Future of JSON API’ is closed to new replies.