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.

Sign up for GitHub

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

Jump to bottom

[DARGA] Display error when the ajax request fails #13690

Merged
merged 1 commit into from Jan 31, 2017
Merged

[DARGA] Display error when the ajax request fails #13690

merged 1 commit into from Jan 31, 2017

Conversation

lgalis
Copy link
Contributor

@lgalis lgalis commented Jan 30, 2017 •
edited by miq-bot

@lgalis
Copy link
Contributor Author

lgalis commented Jan 30, 2017

@miq-bot add_label wip, ui, bug

@lgalis lgalis force-pushed the 5.6_display_error_on_ajax_request_error branch 4 times, most recently from bdc27cd to 1214bba Compare January 30, 2017 16:01
himdel
himdel reviewed Jan 30, 2017
View reviewed changes
app/assets/javascripts/miq_application.js
@@ -790,15 +790,20 @@ function miqAjaxButtonSend(url, serialize_fields) {

// Function to generate an Ajax request
function miqAjax(url, serialize_fields) {
var data = undefined;
var data = 'undefined';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is broken, you really need to initialize to undefined, not 'undefined' otherwise if you don't overwrite it, it will send malformed data to the server - sends a string instead of nothing.

@himdel
Copy link
Contributor

himdel commented Jan 30, 2017 •
edited

..and after that, the reson the specs are failing now is that whenever we mock miqJqueryRequests in the specs, it doesn't return that promise you're expecting.

Specifically, I think you're looking for miq_application_spec.js, line 348 .. it provides a then, but not a catch.

        spyOn(window, 'miqJqueryRequest').and.callFake(function () {
          return {
            then: function (ok, err) {
              err()
            },
            catch: function (err) {
              err()
            },
          };
        });

should help :) (and an identical change in the next describe)

EDIT: fixed

@lgalis lgalis force-pushed the 5.6_display_error_on_ajax_request_error branch 2 times, most recently from 79eb624 to e49d864 Compare January 30, 2017 17:58
@lgalis lgalis force-pushed the 5.6_display_error_on_ajax_request_error branch from e49d864 to 2ff8a4a Compare January 30, 2017 18:02
@himdel
Copy link
Contributor

himdel commented Jan 30, 2017

LGTM 👍

@miq-bot
Copy link
Member

miq-bot commented Jan 30, 2017

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

@lgalis lgalis changed the title [WIP] [DARGA] Display error when the ajax request fails [DARGA] Display error when the ajax request fails Jan 30, 2017
@lgalis
Copy link
Contributor Author

lgalis commented Jan 30, 2017

@miq-bot remove_label wip

@miq-bot miq-bot removed the wip label Jan 30, 2017
@lgalis
Copy link
Contributor Author

lgalis commented Jan 30, 2017

@miq-bot assign @simaishi

@simaishi simaishi merged commit 6efdf6a into ManageIQ:darga Jan 31, 2017
@simaishi simaishi added this to the Sprint 54 Ending Feb 13, 2017 milestone Jan 31, 2017
@simaishi simaishi mentioned this pull request Jan 31, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@himdel himdel himdel left review comments

Assignees

@simaishi simaishi

Labels
bug ui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@lgalis @himdel @miq-bot @simaishi