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 repository CRUD #346

Merged
merged 22 commits into from
Mar 27, 2017
Merged

Conversation

ZitaNemeckova
Copy link
Contributor

@ZitaNemeckova ZitaNemeckova commented Feb 10, 2017

New functionality for Repository in Automation -> Ansible -> Repositories.

Added Configuration button in toolbar with Add/Edit/Delete options.

Add form:
screen shot 2017-03-24 at 2 48 11 pm

Edit form:
screen shot 2017-03-24 at 2 49 16 pm

@miq-bot add_label wip

TODO:

  • Add x Edit
  • Reset
  • Create
  • Read
  • Update
  • Delete from list
  • Delete from summary
  • Get all SCM credentials
  • Double render
  • manager_id
  • Remove http:// prefix in URL

@h-kataria
Copy link
Contributor

@ZitaNemeckova validation message on URL field on form should show in Red, currently it is showing in grey text.

@h-kataria
Copy link
Contributor

h-kataria commented Mar 23, 2017

@ZitaNemeckova title header on the downloaded repo summary should say "Repository"
download_repo_summary

Title header on Edit repository form needs to be fixed too.

message = __("Unable to add Repository ") + vm.repositoryModel.name + " ." + response.results[0].message;
}
else {
message = sprintf(__("Addition of Repository \"%s\" was successfully initialized."), vm.repositoryModel.name);
Copy link
Contributor

Choose a reason for hiding this comment

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

change this to "Add of ..." to make it consistent with other forms in UI

message = __("Unable to update Repository") + vm.repositoryModel.name + " ." + response.message;
}
else {
message = sprintf(__("Update of Repository \"%s\" was successfully initialized."), vm.repositoryModel.name);
Copy link
Contributor

Choose a reason for hiding this comment

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

same here "Edit of ..." to be consistent

AnsibleRepositoryController.model.where(:id => checked).each do |repo|
begin
repo.delete_in_provider_queue
add_flash(_("Deletion of Repository \"%{name}\" was successfully initiated.") % {:name => repo.name})
Copy link
Contributor

Choose a reason for hiding this comment

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

Change this to "Delete of ..."

};

$scope.cancelClicked = function() {
var message = repositoryId === 'new' ? __("Addition of Repository canceled by user.") : sprintf(__("Addition of Repository \"%s\" canceled by user."), vm.repositoryModel.name)
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be "Add of Repository cancelled by user." : "Edit of Repository "%s" cancelled by user."

options_for_select([["#{_('GIT')}", "git"]], 'git'),
"ng-model" => "vm.repositoryModel.scm_type",
:disabled => true)
.form-group
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need to add a class here to get validation message to show in red color
.form-group{"ng-class" => "{'has-error': angularForm.scm_url.$invalid}"}

@h-kataria
Copy link
Contributor

@ZitaNemeckova can you also address code climate errors, most of them look easy to address. I was successfully able to add/edit/delete repositories 👍

@ZitaNemeckova
Copy link
Contributor Author

@h-kataria PDF issue addressed in ManageIQ/manageiq#14485

@ZitaNemeckova ZitaNemeckova force-pushed the repository_form branch 3 times, most recently from 487426f to adfae8a Compare March 24, 2017 12:54
@ZitaNemeckova
Copy link
Contributor Author

Codeclimate failures related to using $scope cannot be addressed as it has to be used to work with buttons. It will be removed once all controller has controllerAs. Avoid using a variable and instead use chaining with the getter syntax. other controllers have it this way. Similar code found in 3 other locations for $scope.cancelClicked cannot be de-duplicated.

@ZitaNemeckova
Copy link
Contributor Author

@h-kataria I will add them in separate PR due to time pressure.

Url validation needed because default ng url validation was positive for any web protocol but it was needed to be positive only for http or https
@gmcculloug
Copy link
Member

@jameswnl Please review. I think you are most familiar with this from the provider side.

:javascript
ManageIQ.angular.app.value('repositoryId', '#{@id}');
miq_bootstrap('#form_div');
$("#form-div").submit(function (e) {
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 since you should never be submitting the form (old-style) .. Better to just put miqSparkleOn() in addClicked and saveClicked.


$scope.addClicked = function() {
API.post('/api/configuration_script_sources/', vm.repositoryModel)
.then(getBack)
Copy link
Contributor

Choose a reason for hiding this comment

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

2 spaces, not three please

};

var validUrl = function(s) {
debugger;
Copy link
Contributor

Choose a reason for hiding this comment

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

wink wink, nudge nudge

@himdel
Copy link
Contributor

himdel commented Mar 24, 2017

(travis failure unrelated - gitter March 24, 2017 5:54 PM)

scm_type: 'git',
manager_resource: {},
scm_url: '',
scm_credentials: null,
Copy link
Contributor

@jameswnl jameswnl Mar 24, 2017

Choose a reason for hiding this comment

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

(Potentially a silly question as I am not familiar with UI code)
There can be 0 or 1 scm_credential associated with a repo/project. Here the pluralized form, does it mean multiple values expected? Or it is the list of values to be selected from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jameswnl there are no stupid questions :) I must have missed some change because scm_credentials no longer exists and there's authentication_id instead. Thanks :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, feel good to be useful in UI land 😄

@ZitaNemeckova ZitaNemeckova changed the title [WIP] Add repository CRUD Add repository CRUD Mar 27, 2017
@ZitaNemeckova
Copy link
Contributor Author

@miq-bot remove_label wip

@miq-bot miq-bot removed the wip label Mar 27, 2017
@miq-bot
Copy link
Member

miq-bot commented Mar 27, 2017

Checked commits ZitaNemeckova/manageiq-ui-classic@a727871~...b905166 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
4 files checked, 9 offenses detected

app/helpers/application_helper/toolbar/ansible_repositories_center.rb

  • ❗ - Line 14, Col 37 - Style/MultilineMethodCallBraceLayout - Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.
  • ❗ - Line 22, Col 37 - Style/MultilineMethodCallBraceLayout - Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.
  • ❗ - Line 31, Col 92 - Style/MultilineMethodCallBraceLayout - Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.
  • ❗ - Line 34, Col 3 - Style/IndentArray - Indent the right bracket the same as the first position after the preceding left parenthesis.
  • ❗ - Line 3, Col 5 - Style/IndentArray - Use 2 spaces for indentation in an array, relative to the first position after the preceding left parenthesis.

app/helpers/application_helper/toolbar/ansible_repository_center.rb

  • ❗ - Line 14, Col 26 - Style/MultilineMethodCallBraceLayout - Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.
  • ❗ - Line 21, Col 92 - Style/MultilineMethodCallBraceLayout - Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.
  • ❗ - Line 24, Col 3 - Style/IndentArray - Indent the right bracket the same as the first position after the preceding left parenthesis.
  • ❗ - Line 3, Col 5 - Style/IndentArray - Use 2 spaces for indentation in an array, relative to the first position after the preceding left parenthesis.

@h-kataria
Copy link
Contributor

@ZitaNemeckova merging this PR, please add jasmine spec tests in an additional PR.

@h-kataria h-kataria merged commit 9e406c0 into ManageIQ:master Mar 27, 2017
@h-kataria h-kataria added this to the Sprint 57 Ending Mar 27, 2017 milestone Mar 27, 2017
@ZitaNemeckova ZitaNemeckova deleted the repository_form branch September 12, 2017 14:54
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.

8 participants