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

Add EmbeddedTerraform UI #3

Merged
merged 12 commits into from
Apr 24, 2024
Merged

Conversation

agrare
Copy link
Member

@agrare agrare commented Mar 4, 2024

Depends on:

Depend
TODO

  • Routes
  • Menu Items
  • db_to_controller
  • title_from_layout
  • WorkflowRepositoryForm, WorkflowCredentialsForm javascript forms

Menu:
image

Credentials Show List:
image

Credentials Show:
image

Credentials Show -> Repositories Show List:
image

Repositories Show List:
image

Repositories Show:
image

Repositories Show -> Templates Show List:
image

Templates Show List:
image

Templates Show:
image

@agrare

This comment was marked as outdated.

@GilbertCherrie
Copy link
Member

This pr depends on UI PR: ManageIQ/manageiq-ui-classic#9117 and core PR: ManageIQ/manageiq#22943

@jrafanie
Copy link
Member

Note, #9 has been merged, renaming ConfigurationScriptPayload to Template

menu_section :embedded_terraform_workflow

def self.model
ManageIQ::Providers::EmbeddedTerraform::AutomationManager::ConfigurationScriptPayload
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ManageIQ::Providers::EmbeddedTerraform::AutomationManager::ConfigurationScriptPayload
ManageIQ::Providers::EmbeddedTerraform::AutomationManager::Template

locale/en.yml Outdated Show resolved Hide resolved
@agrare agrare force-pushed the add_embedded_terraform_ui branch 2 times, most recently from d617432 to 24786f6 Compare March 28, 2024 19:21
@agrare
Copy link
Member Author

agrare commented Mar 28, 2024

Okay it was pretty involved to rename this but I think I got everything done and tested

@agrare agrare changed the title [WIP] Add EmbeddedTerraform UI Add EmbeddedTerraform UI Mar 28, 2024
@agrare
Copy link
Member Author

agrare commented Mar 28, 2024

I'm going to take this out of WIP, I've tested creating SCM credentials, creating a repository, and viewing imported templates on the UI and everything appears functional.

when 'ansible_credential_tag'
tag(self.class.model)
when "ansible_repository_tag" # repositories from nested list
tag(ManageIQ::Providers::EmbeddedTerraform::AutomationManager::ConfigurationScriptSource)
Copy link
Member

Choose a reason for hiding this comment

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

Do each of these cases automatically do assert_privileges checks from tag, javascript_redirect, etc.?

Copy link
Member

Choose a reason for hiding this comment

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

🤣 I think they do... looking down below.

Copy link
Member

Choose a reason for hiding this comment

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

not sure about tag but maybe

Copy link
Member

Choose a reason for hiding this comment

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

should it say ansible_credential_tag and ansible_resository_tag?

end

def tag_edit_form_field_changed
assert_privileges('ansible_credential_tag')
Copy link
Member

Choose a reason for hiding this comment

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

similar to above, is it ansible or terraform?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we don't want to depend something which requires a database migration in order to merge this, so for now ansible_credential_tag is fine (this is the same for embedded_workflows which also uses ansible_credential_tag and we can fix them together)

page << javascript_prologue
page.replace("main_div", :template => "embedded_terraform_repository/show")
end
when "ansible_repository_tag"
Copy link
Member

Choose a reason for hiding this comment

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

same as above re: ansible instead of terraform

t,
:items => [
button(
:ansible_credential_tag,
Copy link
Member

Choose a reason for hiding this comment

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

Ditto from above

:onwhen => "1+",
:items => [
button(
:ansible_credential_tag,
Copy link
Member

Choose a reason for hiding this comment

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

Same

:items =>
[
button(
:ansible_repository_tag,
Copy link
Member

Choose a reason for hiding this comment

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

more ansible product features

:items =>
[
button(
:ansible_repository_tag,
Copy link
Member

Choose a reason for hiding this comment

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

last two ansible features

jrafanie added a commit to jrafanie/manageiq that referenced this pull request Apr 1, 2024
We started with embedded ansible but with the addition of embedded terraform,
it makes sense to generalize these common product features.

Related to ManageIQ/manageiq-providers-embedded_terraform#3
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Apr 1, 2024
We started with embedded ansible but with the addition of embedded terraform,
it makes sense to generalize these common product features.

Related to ManageIQ/manageiq-providers-embedded_terraform#3
@agrare agrare force-pushed the add_embedded_terraform_ui branch from a0ebda0 to 8ede6a6 Compare April 1, 2024 19:23
@nasark nasark mentioned this pull request Apr 16, 2024
2 tasks
@agrare
Copy link
Member Author

agrare commented Apr 24, 2024

@jrafanie I think this is ready to merge we just wont have ManageIQ/manageiq-ui-classic#9117 until we add this plugin to the core Gemfile

@jrafanie
Copy link
Member

@jrafanie I think this is ready to merge we just wont have ManageIQ/manageiq-ui-classic#9117 until we add this plugin to the core Gemfile

yes, I tried it out yesterday and it was working so let's get this in. It's in the new provider anyway so you would need to have added the provider in the first place to encounter any problems this PR may introduce.

@jrafanie jrafanie assigned jrafanie and unassigned jeffibm Apr 24, 2024
@jrafanie
Copy link
Member

@jeffibm feel free to review/comment on this after it's merged, we can always fix any problems you see. Thanks!

@jrafanie jrafanie merged commit 3c41cd3 into ManageIQ:master Apr 24, 2024
3 of 4 checks passed
@agrare agrare deleted the add_embedded_terraform_ui branch April 24, 2024 16:21
jrafanie added a commit to jrafanie/manageiq that referenced this pull request May 2, 2024
We started with embedded ansible but with the addition of embedded terraform,
it makes sense to generalize these common product features.

Related to ManageIQ/manageiq-providers-embedded_terraform#3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants