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

system groups - introduce system group event history in the UI #199

Merged
merged 26 commits into from
Jun 15, 2012

Conversation

bbuckingham
Copy link
Member

This pull request contains changes to add to the UI system group event history. Prior to this request, users could schedule actions on a system group (e.g. pkg install); however, they did not have a way to view the history of those actions in the UI.

With these changes, a user will navigate to System Groups -> [groupX] -> Details -> Events and they may see the list of "completed" or "in progress" events/actions for the group. For any in progress actions, the status will be updated as they complete. The user may also select an action/event to see details on it. If that action has any systems that are still processing the action (i.e. in progress), status polling will continue until completion. When the action is completed, the user can hover on an icon for the system to specific details associated with that request.

With this merge, we are eliminating the system_task model and replacing it with a polymorphic association between task_status and the owner (e.g. system). This will make it easier for future users of the task_status, but with this change comes the need for developers to run "db:migrate" to update the schema. Also, since we are removing the system_task model, this merge requires changes to any existing users (e.g. system packages, system errata...etc).

This makes Content as a 3rd level nav consisting of Packages and Errata,
consistent with the Systems page.
This commit contains minor refactorings for some code that will be
shared for system groups.
This initial commit contains some of the pieces needed to support
system group event history (e.g. nav/controller/model/view...etc).

There will be additional commits to complete this logic.  This
includes covering things like status polling from event history
pane, listing of systems on a given event (along w/ status)...etc.
This commit removes the current system_tasks model and associations
between a system and task status.  It replaces it with a polymorphic
association on the task_status.  That association adds a 'task_owner'
to the task status, which any other model can be associated with (e.g. System).
This allows us to associate a task essentially with any other model
without requiring an additional join table (like the current
system_task).

Updated all existing code using system_task to use this new relationship.

Also, created a migration which will add the elements needed for
the relationship (task_owner_type, task_owner_id) to the model,
populate it with appropriate data based upon existing system_tasks
records and then drop the system_tasks.
The the unbind that was being done for the +Add and -Remove link
was not correctly unbinding the event.  As a result, it was
possible that multiple events would be generated when the user
clicked those links.  This commit addresses that issue.
Currently using the .spinner on these views does not generated a desired
affect.  Essentially, a div is added causing spinner and status text
to be on separate lines.  For now, to avoid this, going to use the
image_tag direct.  If the issue w/ .spinner class is resolved, this
commit can be reverted.
This is an initial commit to show the status of completed/failed
task associated with a system group action, via a tipsy.  This will be updated
when status polling is added to add the tipsy when the task reaches completion.
This commit contains changes to support polling for status
of jobs scheduled on a system group.  This allows user to
see status of in progress jobs from the SystemGroups -> Group -> Events
pane.
This commit contains changes so that if the user has
navigated to SystemGroups->[groupX]->Events->[jobY],
the user's client will poll for updated status for any
pending tasks for that current job.

In order to support this, updated the system_events poller
to support polling for both jobs and tasks at the same time.
This minimizes the polling requests.

Note: for the Systems page, this means we'd use tasks, but
for System Groups page, we use jobs+tasks.
As part of getting rid of system_task.rb, the system_ids field
was removed from the index.  In order to support searching
for tasks based on system, we can now use the task owner id field.
This commit contains changes to provide basic search support
for system group event history.  This includes searching based
on username and parameters.  In a future commit, support for search
by status will be added; however, that requires a bit more work,
based on the fact that status is determined by looking across all
tasks related to the job and the way system events currently handle it.
@@ -23,7 +23,9 @@
"save": '#{escape_javascript(_("Save"))}',
"saving": '#{escape_javascript(_("Saving..."))}',
"close": '#{escape_javascript(_("Close"))}',
"x_of_y" : function(x,y,z){ return '#{_("Viewing %X of %Y results (%Z Total)")}'.replace("%X", x).replace("%Y", y).replace("%Z", z); },
"x_of_y_total" : function(x,y,z){ return '#{_("Viewing %X of %Y results (%Z Total)")}'.replace("%X", x).replace("%Y", y).replace("%Z", z); },
"x_of_y" : function(x,y){ return '#{_("Viewing %X of %Y results")}'.replace("%X", x).replace("%Y", y); },
Copy link
Member

Choose a reason for hiding this comment

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

Appears we may have some inconsistency around the usage of escape_javascript and our i18n strings in this file both from the ones that appear to be added with this commit and lines 29-31 below.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. A little bit of copy/paste here. Updated with commit 0170431

@ehelms
Copy link
Member

ehelms commented Jun 13, 2012

Other than the previous single line comment this request is an ACK from me.

@@ -38,10 +53,17 @@ def refresh_for_owner(owner)
'INNER JOIN job_tasks ON job_tasks.task_status_id = task_statuses.id').joins(
'INNER JOIN jobs ON jobs.id = job_tasks.job_id')

# refresh those tasks to get latest status
ids = tasks.collect{|row| row[:id]}
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor suggestion -> can ya do

ids = tasks.select(:id).collect{|row| row[:id]}

Tasks table has a lot of columns we are only interested in one column i.e. id.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice find, @parthaa

Copy link
Member Author

Choose a reason for hiding this comment

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

Good suggestion. Updated w/ commit d68a667

@knowncitizen
Copy link
Contributor

Ack from me, assuming the merge in from master goes as planned...

Updating a couple of method names based on pull request comments.
knowncitizen added a commit that referenced this pull request Jun 15, 2012
system groups - introduce system group event history in the UI
@knowncitizen knowncitizen merged commit 4094241 into Katello:master Jun 15, 2012
@knowncitizen
Copy link
Contributor

Thanks for updating the helptip!

ehelms pushed a commit that referenced this pull request Apr 27, 2013
system groups - introduce system group event history in the UI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants