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

Dialog Editor, ExplorerPresenter - make sure all ids are strings in javascript #6388

Merged
merged 3 commits into from
Nov 15, 2019
Merged

Dialog Editor, ExplorerPresenter - make sure all ids are strings in javascript #6388

merged 3 commits into from
Nov 15, 2019

Conversation

himdel
Copy link
Contributor

@himdel himdel commented Nov 7, 2019

for regions > 9006, our bigint ids become floats in javascript, and can be off-by-one. (Original issue: #1405)

This makes sure the dialog editor works in those regions:

  • the dialog_id_action change fixes the result of the requestDialogId dialog editor controller method, controlling where to load the dialog data from
  • the ExplorerPresenter change fixes the toolbar (Edit) button failing (becuase it uses ManageIQ.record.recordId, which was set to a number otherwise)
    • (the infra_networking & vm_common change only remove the now extra to_s because it's handled in ExplorerPresenter now)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1769968

for regions > 9006, our bigint ids become floats in javascript, and can be off-by-one.

This makes sure dialog editor works in those regions.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1769968
…nless nil

Most of the code setting `presenter[:record_id]` or `:parent_id` is not doing the `.to_s` conversion necessary for javascript,
so ExplorerPresenter should do it.

(Also removes the now extra .to_s from the 2 calls that were actually right before.)

Fixes toolbar Edit button not working on a service dialog detail screen.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1769968
@himdel himdel changed the title [WIP] Dialog Editor - make sure all ids are strings in javascript [WIP] Dialog Editor, ExplorerPresenter - make sure all ids are strings in javascript Nov 14, 2019
@himdel
Copy link
Contributor Author

himdel commented Nov 14, 2019

@miq-bot remove_label wip

@miq-bot miq-bot changed the title [WIP] Dialog Editor, ExplorerPresenter - make sure all ids are strings in javascript Dialog Editor, ExplorerPresenter - make sure all ids are strings in javascript Nov 14, 2019
@miq-bot miq-bot removed the wip label Nov 14, 2019
@himdel
Copy link
Contributor Author

himdel commented Nov 14, 2019

Cc @martinpovolny (ExplorerPresenter id changes :))

@miq-bot
Copy link
Member

miq-bot commented Nov 14, 2019

Checked commits https://github.com/himdel/manageiq-ui-classic/compare/f162d770786f02d05c9ce35d6b27ebb41bdad4ea~...4dfad619acff1fcfa08215c350a6d24a83f5b314 with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
5 files checked, 0 offenses detected
Everything looks fine. ⭐

@martinpovolny martinpovolny added this to the Sprint 125 Ending Nov 25, 2019 milestone Nov 15, 2019
@martinpovolny martinpovolny merged commit 747eb57 into ManageIQ:master Nov 15, 2019
@himdel himdel deleted the bz1769968-dialog-editor-id branch November 15, 2019 13:10
simaishi pushed a commit that referenced this pull request Nov 15, 2019
Dialog Editor, ExplorerPresenter - make sure all ids are strings in javascript

(cherry picked from commit 747eb57)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1772932
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit ecde4ce9f8dad73f0cccb58ae4d45440a1322ce1
Author: Martin Povolny <[email protected]>
Date:   Fri Nov 15 08:17:41 2019 +0100

    Merge pull request #6388 from himdel/bz1769968-dialog-editor-id
    
    Dialog Editor, ExplorerPresenter - make sure all ids are strings in javascript
    
    (cherry picked from commit 747eb5796e4a6c2e719eaebe1d253808bf708a0b)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1772932

simaishi pushed a commit that referenced this pull request Dec 16, 2019
Dialog Editor, ExplorerPresenter - make sure all ids are strings in javascript

(cherry picked from commit 747eb57)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1772933
@simaishi
Copy link
Contributor

Ivanchuk backport details:

$ git log -1
commit f3bb762bc7212ba46f0ebc757459de5ca4f03202
Author: Martin Povolny <[email protected]>
Date:   Fri Nov 15 08:17:41 2019 +0100

    Merge pull request #6388 from himdel/bz1769968-dialog-editor-id

    Dialog Editor, ExplorerPresenter - make sure all ids are strings in javascript

    (cherry picked from commit 747eb5796e4a6c2e719eaebe1d253808bf708a0b)

    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1772933

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants