Set afterGet when the OPTIONS data is actually retrieved

[AparnaKarve]

API calls are async and therefore afterGet needs to be set once we have actually retrieved the data in getCredentialOptions from the API.options('/api/authentications') call.

Without this change, there is a chance that one could see an empty dropdown for a few brief seconds when the form is loaded as seen in the screenshot --

[AparnaKarve]

@miq-bot add_label fine/yes, bug

[AparnaKarve]

/cc @mzazrivec @himdel @h-kataria Please review.

Noticed the issue in yesterday's heavy-duty testing.

[miq-bot]

Checked commit AparnaKarve@4f2ee61 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
0 files checked, 0 offenses detected
Everything looks good. 🏆

[simaishi]

Fine backport details:

$ git log -1
commit 6847a255821569beab1770f0a4cbb937d7c2d248
Author: Milan Zázrivec <[email protected]>
Date:   Wed Mar 29 10:41:03 2017 +0200

    Merge pull request #827 from AparnaKarve/ansible_empty_dropdown
    
    Set afterGet when the OPTIONS data is actually retrieved
    (cherry picked from commit 0b36dcb4380cd457ba5078b0366f8076ed671a10)

[himdel]

@AparnaKarve This breaks afterGet when the OPTIONS request responds faster than the API.get request for the edit code... seems like $q.all to wait on multiple promises may be your friend here :)

[AparnaKarve]

@himdel #855 might be an answer to the edit case.

[himdel]

Not really, since that one deals with a different request entirely :) But thanks for that, that's exactly how I found this bug :).

But.. since more info is probably needed, the bug is this:

1. we're editing something; afterGet == false
2. API.options('/api/authentications') is called (and will eventually call getCredentialOptions)
3. API.get('/api/authentications' + credentialId) is called (and will eventually call getCredentialFormData)
4. if the options request has responded first, getCredentialsOptions will get called, causing afterGet = true, before getCredentialFormData was called, so we render the form without any data (except for the select options) => BUG

OTOH if the get request has responded first, getCredentialFormData is called, causing afterGet = true, causing the form to get rendered with all data except for the select options. Since you fixed exactly this for the new code path, I assume you'd consider this a bug as well.

The only way to make sure afterGet will be set to true only after both these requests have succeeded, you'd have to do essentially this...

    var select_promise = API.options('/api/authentications')
      .then(getCredentialOptions)
      .catch(miqService.handleFailure);

    var data_promise;
    if (credentialId !== 'new') {
      data_promise = API.get('/api/authentications/' + credentialId)
        .then(getCredentialFormData)
        .catch(miqService.handleFailure);
    } else {
       data_promise = Promise.resolve();
       // ...the rest of the else body...
    }

    $q.all([data_promise, select_promise])
      .then(function() {
        vm.afterGet = true;
      });

(and remove the other two afterGet = true)

[AparnaKarve]

Ok, so either use $q.all or just good old chaining, i.e. handle the edit part of the code in getCredentialOptions.

What do you prefer?

[himdel]

Definitely $q.all since chaining means waiting for 2 request one after the other, while with $q.all, these requests can still happen in parallel.

Thanks for looking into this! :)

[AparnaKarve]

Alrighty, you got it! :)