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

Update Cloud Image View to Differentiate Between Snapshots and Non-Snapshots #12970

Conversation

mansam
Copy link
Contributor

@mansam mansam commented Dec 2, 2016

This PR updates the Cloud Image view with a Type column that identifies Cloud Images as Snapshots if they descend from a parent VM.

screenshot at 2016-12-02 15 51 48

@tzumainn


def snapshot_or_image
genealogy_parent.nil? ? _("Image") : _("Snapshot")
end
Copy link
Member

Choose a reason for hiding this comment

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

This looks like UI decorator code. I think the model can add boolean predicates -- snapshot? and image? methods. These can then be consumed by UI code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

UI code now handles what to display based on the predicate methods.

@mansam mansam force-pushed the update-image-view-to-differentiate-between-snapshots-and-non-snapshots branch 2 times, most recently from b88b446 to 5425e6d Compare December 9, 2016 19:19
@miq-bot
Copy link
Member

miq-bot commented Dec 22, 2016

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

@chessbyte chessbyte added the wip label Jan 3, 2017
@mansam mansam force-pushed the update-image-view-to-differentiate-between-snapshots-and-non-snapshots branch from 5425e6d to fd0fcbc Compare January 10, 2017 17:51
@mansam
Copy link
Contributor Author

mansam commented Jan 10, 2017

This is now just the predicates and the changes to the yaml view. The controller changes are being moved to the new UI repo.

@@ -24,6 +24,7 @@ cols:
- allocated_disk_storage
- last_scan_on
- region_description
- image?
Copy link
Member

Choose a reason for hiding this comment

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

Can you do a white-space cleanup on this file to remove these ^M characters?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can. I'm not sure why this file (and several other of these yamls) seems to have windows style line endings.

@miq-bot
Copy link
Member

miq-bot commented Jan 25, 2017

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

@tzumainn
Copy link
Contributor

@mansam my fault for not testing this in quite a while! I couldn't quite see the changes in the UI; I'm looking at the "Provision Instances - Select an Image" page, but let me know if I'm looking in the wrong place!

@mansam mansam force-pushed the update-image-view-to-differentiate-between-snapshots-and-non-snapshots branch from 06de802 to ba7d4d1 Compare March 7, 2017 21:13
@miq-bot
Copy link
Member

miq-bot commented Mar 7, 2017

Checked commits mansam/manageiq@9be6d7c~...ba7d4d1 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks good. 🍪

@tzumainn
Copy link
Contributor

tzumainn commented Mar 7, 2017

👍 verified this updates the image view correctly

@tzumainn
Copy link
Contributor

tzumainn commented Mar 7, 2017

@miq-bot assign @blomquisg

@miq-bot miq-bot assigned blomquisg and unassigned tzumainn Mar 7, 2017
@h-kataria
Copy link
Contributor

@mansam not sure why it is showing product/views/ManageIQ_Providers_CloudManager_Template.yaml whole file as changed, can you check your editor settings. cc @tzumainn

@mansam
Copy link
Contributor Author

mansam commented Mar 13, 2017

@h-kataria I was asked by @chessbyte to change the pre-existing line endings in that file, which should explain why the diff includes the whole file. See the comments here: #12970 (comment)

@h-kataria
Copy link
Contributor

@mansam sorry i didn't notice that.

@mansam
Copy link
Contributor Author

mansam commented Mar 13, 2017

@h-kataria no worries, github did a good job of hiding those comments :)

@blomquisg blomquisg merged commit 8f29231 into ManageIQ:master Mar 15, 2017
@blomquisg blomquisg added this to the Sprint 57 Ending Mar 27, 2017 milestone Mar 15, 2017
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.

6 participants