http helpers for react, API to new js #3662

himdel · 2018-03-20T19:31:27Z

This moves our API code from miq_api.js to app/javascript, converts it a more modern style, and splits the implementation into API-specific parts (http_api/api.js) and the common code (http_api/fetch.js - a miqFetch wrapper around the fetch API), making it possible to use that common code to implement a simple http library (http_api/http.js) for accessing rails controllers with the same interface.

This PR keeps all the existing names for the API code for backward compatibility (will remove in a separate PR, once all the code is converted), but changes the preferred default:

window.API is the API access code for any plain js / react code
(window.vanillaJsAPI is a deprecated name for the same)
$API is the angular service to acccess the API
(API is a deprecated name for the same)
window.http is the http access code for any plain js / react code
(until now, the only ways were angular $http or miqJqueryRequest)
$http is the standard angular service, configured to be usable for miq pruposes
(no changes there)

All of these methods should behave the same way:

when an authentication failure occurs, redirect to login screen (unless the skipLoginRedirect option is used)
when a non-200 response (other than 401) occurs, show the error modal (unless the skipErrors option is used)

~~(So as not to overload codeclimate, this also fixes the eslint config to respect the root options - the airbnb config is still used, it just respects everything set in the root .eslintrc as well.)~~

(Please ignore codeclimate for the API code, this will be fixed in #3929.)

ohadlevy · 2018-03-21T08:15:27Z

please also see https://github.com/priley86/miq_v2v_ui_plugin/blob/master/app/javascript/common/API.js - ideally it should all be combined?

my main concern is around mocking the api calls, /cc @priley86

miq-bot · 2018-03-21T10:07:39Z

This pull request is not mergeable. Please rebase and repush.

himdel · 2018-04-09T17:02:07Z

Yes, the API.js file should not even exist in v2v, ideally v2v should be doing const API = window.API and use that (after this PR that is).

AllenBW · 2018-05-07T17:25:34Z

V2V has skin in this game, bumping for a lil 🏩 😻

himdel · 2018-05-10T12:32:38Z

Looks like this works now, so just bear with me please @AllenBW while I clean this up and create some nice commits :).

AllenBW · 2018-05-10T12:36:48Z

OH EM GEEE MY HERO!!! <3 <3 <3 🙇‍♀️ will always 🐻 with yah @himdel 😋 this is good news.. i'll cool my jets on da wip-pr till this work shows up

himdel · 2018-05-11T15:57:37Z

the following (public) methods are provided for API (this did not change, just mentioning)

API.delete(url, [options])
API.get(url, [options])
API.options(url, [options])
API.post(url, data, [options])
API.put(url, data, [options])

the following methods are provded for http - these are only meant for JSON access points:

http.get(url, [options])
http.post(url, data, [options])

EDIT: added options to http methods

AllenBW · 2018-05-11T16:00:29Z

☝️ the difference being support for options, vs not?

himdel · 2018-05-11T16:04:28Z

@AllenBW not really, it's just we have no options to support there yet - it does not make sense to do skipErrors when talking to rails controllers since the errors will be a 200 response with a rails error html, and the same for skipLoginRedirect, as no meaningful response will happen without being logged in

EDIT: (The imporant difference is that http sends the CSRF token, while API sends the api login token)

EDIT2: and the http option support may make some sense for transformResponse, adding

…ckend the error modal code needs to be able to tell whether the error comes from a $http request or from an API request, and uses the `source` field for that (`API` vs `$http`) to unify http & API access accross the codebase, including react code, we also need a non-angular http access code this new code will be using the fetch API, the same as our API code - which means we'd have 3 source values, two of which share parts of the implementation so .. splitting this into `source = 'fetch' | '$http'` and `backendName = <human string>`

just a straight move, with the bare minimum changes leaving the angular bits in the old code for now, because of potential ordering issues

…from API where needed

should not actually change anything, just make sure var is not used, globals are properly "imported", etc.

…on around them the idea is that miqFetch can become the common implementation, being able to handle logouts, errors, and tokens for both rails http access and rest api

…r the hood the interface is very simple right now, http.get(url) and http.post(url, data) both the functions return a Promise, both handle logouts and errors

the idea behind vanillaJsAPI was that since we did angular first, using the angular service was preferred, thus a short name was needed they still have to have different names so that we can catch the non-angular version being used in angular code but since the codebase is moving towards vanilla JS / React code first, it makes sense for that version to get the short name - API the angular version will be called $API from now, but any code using API will keep working until we update it all to the new state (a second PR will change API to $API in existing angular code and remove the old names completely once there are no users)

himdel · 2018-05-11T16:12:32Z

Cc @martinpovolny

modelled after $http transformResponse, this can be a custom function to change the response in any way needed this will only be applied to success responses, not on error

himdel · 2018-05-11T16:28:20Z

Added transformResponse option support, so that v2v API requests can do something like...

API.get('/foo', { transformResponse: (o) => ({ data: o })}) to get the data in the same format as before.

not strictly useful right now, but since the same set of options is supported between the API and http code, it makes sense to make it possible to use some some day

because jest tests don't load any packs first, we need to set `window.vanillaJsAPI` before using it now

miq-bot · 2018-05-14T11:47:12Z

Checked commits https://github.com/himdel/manageiq-ui-classic/compare/91793f5613018cacffb2074dfeda93942b3ea170~...8edc838bd9ffea54d1241fb588c872c5620b6356 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🏆

AllenBW · 2018-05-15T22:13:47Z

soooooooo we now have a blocker that could use this pr merged 😢 https://bugzilla.redhat.com/show_bug.cgi?id=1573547

AllenBW · 2018-05-16T11:44:14Z

BEST MORNING NEWS EVA <3 📰 👍

martinpovolny · 2018-05-16T12:27:33Z

;-)

himdel · 2018-05-30T15:11:13Z

@miq-bot add_label gaprindashvili/yes, transformation
@miq-bot remove_label gaprindashvili/no

@simaishi This can not be backported directly, depends on the whole es6 support ecosystem and several npm (instead of bower) dependencies => #4005

simaishi · 2018-06-01T12:43:30Z

Backported to Gaprindashvili via #4005

miq-bot added the wip label Mar 20, 2018

miq-bot added the unmergeable label Mar 21, 2018

himdel mentioned this pull request May 4, 2018

Timed Out migration screen keeps repeatedly prompting for login ManageIQ/manageiq-v2v#283

Closed

miq-bot removed the unmergeable label May 10, 2018

himdel added enhancement and removed wip labels May 11, 2018

himdel changed the title ~~[WIP] http helpers for react, API to new js~~ http helpers for react, API to new js May 11, 2018

himdel added 8 commits May 11, 2018 16:07

move miq_api.js (non-angular) to app/javascript

9afc9bf

just a straight move, with the bare minimum changes leaving the angular bits in the old code for now, because of potential ordering issues

base64encode compat code - doesn't need to be a pack anymore, import …

1e73d80

…from API where needed

miq_api - es6ify code

3081e7a

should not actually change anything, just make sure var is not used, globals are properly "imported", etc.

miq_api - move helper functions to fetch.js, create a miqFetch functi…

d3b32cd

…on around them the idea is that miqFetch can become the common implementation, being able to handle logouts, errors, and tokens for both rails http access and rest api

add http - http request interface for react code - uses miqFetch unde…

bf1d91b

…r the hood the interface is very simple right now, http.get(url) and http.post(url, data) both the functions return a Promise, both handle logouts and errors

http_api/index.js - import and export http & API code

3b4eead

miqFetch - support a transformResponse option

4dbca77

modelled after $http transformResponse, this can be a custom function to change the response in any way needed this will only be applied to success responses, not on error

himdel added 2 commits May 11, 2018 16:33

http.{get,post} - add optional options support

4f632ac

not strictly useful right now, but since the same set of options is supported between the API and http code, it makes sense to make it possible to use some some day

Fix sendDataWithRx is not a function - bad import

0871c4c

AllenBW mentioned this pull request May 11, 2018

Use window api methods for correct session timeout ManageIQ/manageiq-v2v#303

Merged

jest - Fix ReferenceError: vanillaJsAPI is not defined

8edc838

because jest tests don't load any packs first, we need to set `window.vanillaJsAPI` before using it now

martinpovolny merged commit fda046b into ManageIQ:master May 16, 2018

martinpovolny added this to the Sprint 86 Ending May 21, 2018 milestone May 16, 2018

martinpovolny self-assigned this May 16, 2018

martinpovolny added the gaprindashvili/no label May 16, 2018

himdel deleted the http-api-i3658 branch May 16, 2018 14:26

This was referenced May 22, 2018

Use http, not API for querying rails controller endpoints ManageIQ/manageiq-v2v#340

Merged

v2v in gaprindashvili #3963

Closed

http - send cookie #3978

Merged

himdel mentioned this pull request May 30, 2018

[GAPRINDASHVILI] API transformRequest support + http service #4005

Merged

miq-bot added gaprindashvili/yes v2v and removed gaprindashvili/no labels May 30, 2018

simaishi added gaprindashvili/backported and removed gaprindashvili/yes labels Jun 1, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

http helpers for react, API to new js #3662

http helpers for react, API to new js #3662

himdel commented Mar 20, 2018 •

edited

Loading

ohadlevy commented Mar 21, 2018 •

edited

Loading

miq-bot commented Mar 21, 2018

himdel commented Apr 9, 2018 •

edited

Loading

AllenBW commented May 7, 2018

himdel commented May 10, 2018 •

edited

Loading

AllenBW commented May 10, 2018

himdel commented May 11, 2018 •

edited

Loading

AllenBW commented May 11, 2018

himdel commented May 11, 2018 •

edited

Loading

himdel commented May 11, 2018

himdel commented May 11, 2018 •

edited

Loading

miq-bot commented May 14, 2018

AllenBW commented May 15, 2018

AllenBW commented May 16, 2018

martinpovolny commented May 16, 2018

himdel commented May 30, 2018

simaishi commented Jun 1, 2018

http helpers for react, API to new js #3662

http helpers for react, API to new js #3662

Conversation

himdel commented Mar 20, 2018 • edited Loading

ohadlevy commented Mar 21, 2018 • edited Loading

miq-bot commented Mar 21, 2018

himdel commented Apr 9, 2018 • edited Loading

AllenBW commented May 7, 2018

himdel commented May 10, 2018 • edited Loading

AllenBW commented May 10, 2018

himdel commented May 11, 2018 • edited Loading

AllenBW commented May 11, 2018

himdel commented May 11, 2018 • edited Loading

himdel commented May 11, 2018

himdel commented May 11, 2018 • edited Loading

miq-bot commented May 14, 2018

AllenBW commented May 15, 2018

AllenBW commented May 16, 2018

martinpovolny commented May 16, 2018

himdel commented May 30, 2018

simaishi commented Jun 1, 2018

himdel commented Mar 20, 2018 •

edited

Loading

ohadlevy commented Mar 21, 2018 •

edited

Loading

himdel commented Apr 9, 2018 •

edited

Loading

himdel commented May 10, 2018 •

edited

Loading

himdel commented May 11, 2018 •

edited

Loading

himdel commented May 11, 2018 •

edited

Loading

himdel commented May 11, 2018 •

edited

Loading