Problem/Motivation

The job icons are not correctly representing the situation.

The hourglass (blue) is displayed for unsubmitted jobs. A job manager though isn't waiting for anything here.

The orange triangle is (also) displayed for a submitted job. This state doesn't need action from the job manager as long as no unreviewed items are pending.

Proposed resolution

Switch the unsubmitted jobs to a red triangle. (Or any better proposal...)
A submitted job should display the hourglass as long as no item is pending for review.

User interface changes

Icons fit better.

CommentFileSizeAuthor
#17 2663756-fix.patch3.25 KBjohnchque
#13 interdiff-2663756-9-13.txt5.34 KBjohnchque
#13 make_job_icons_also-2663756-13.patch15.3 KBjohnchque
#9 interdiff-2663756-2-8.txt13.95 KBjohnchque
#9 make_job_icons_also-2663756-9.patch14.44 KBjohnchque
#5 Screen Shot 2016-02-21 at 22.26.07 .png15.91 KBmiro_dietiker
#5 Screen Shot 2016-02-21 at 22.18.47 .png17.84 KBmiro_dietiker
#2 Screenshot from 2016-02-08 14:50:48.png33.22 KBjohnchque
#2 replace_icons_of_jobs-2663756-2.patch1.8 KBjohnchque
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

miro_dietiker created an issue. See original summary.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
CreditAttribution: johnchque at MD Systems GmbH commented
Issue summary: View changes
Status: Active » Needs review
FileSize
replace_icons_of_jobs-2663756-2.patch1.8 KB
Screenshot from 2016-02-08 14:50:48.png33.22 KB

Icons changed.
About the feedback:

A submitted job should display the hourglass as long as no item is pending for review.

I am not sure if we should do it in that way since the defined state for active jobs is STATE_ACTIVE and it is independent from JobItemState.

The icons now look like this:

miro_dietiker’s picture

miro_dietiker
Location Switzerland
CreditAttribution: miro_dietiker at MD Systems GmbH commented

Discussed with Berdir, we want to have an icon that has multiple variants for the active state.
The internal state names are not so relevant. We could change "Job state" to "Label" and just change "Active" to "In progress" and add the extra item for "Needs review".

miro_dietiker’s picture

miro_dietiker
Location Switzerland
CreditAttribution: miro_dietiker at MD Systems GmbH commented
Status: Needs review » Needs work
miro_dietiker’s picture

miro_dietiker
Location Switzerland
CreditAttribution: miro_dietiker at MD Systems GmbH commented
Issue summary: View changes
FileSize
Screen Shot 2016-02-21 at 22.18.47 .png17.84 KB
Screen Shot 2016-02-21 at 22.26.07 .png15.91 KB

Pushing this.
Identified this inconsistent output after introducing the progress bar.

After applying it looks better.

But the last icon is wrong now. It should show the orange triangle. It's translated, we are not waiting anymore.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
CreditAttribution: miro_dietiker at MD Systems GmbH commented
Title: Replace icons of jobs » Make job icons also represent job item progress
johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
CreditAttribution: johnchque at MD Systems GmbH commented

I am still working on this but it needs a lot of refactoring on tests. I will upload a patch soon.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
CreditAttribution: miro_dietiker at MD Systems GmbH commented

Yeah, discussed this today and identified todo's in individual tests to cover all missing pieces and also upcoming issues... and also identified how we want to split the larger spaghetti in testCheckoutForm(). Most importantly, we said we want to have the overview and exposed filter test coverage with the different job + item states in a separate test method and check icon, progress indicators and also job item / data item status on the related detail level.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
CreditAttribution: johnchque at MD Systems GmbH commented
FileSize
make_job_icons_also-2663756-9.patch14.44 KB
interdiff-2663756-2-8.txt13.95 KB

Added tests. I still thing that we need to delete some lines on other tests that are already being testes on the new ones in the patch.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
CreditAttribution: johnchque at MD Systems GmbH commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 9: make_job_icons_also-2663756-9.patch, failed testing.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
CreditAttribution: miro_dietiker at MD Systems GmbH commented

Looks pretty nice :-) ... can i haz passing tests? ;-)

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
CreditAttribution: johnchque at MD Systems GmbH commented
Status: Needs work » Needs review
FileSize
make_job_icons_also-2663756-13.patch15.3 KB
interdiff-2663756-9-13.txt5.34 KB

My bad, should work now. Also, made some refactoring.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
CreditAttribution: miro_dietiker at MD Systems GmbH commented
Status: Needs review » Fixed

Yay, committed, nice work!

Status: Fixed » Needs work

The last submitted patch, 13: make_job_icons_also-2663756-13.patch, failed testing.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
CreditAttribution: johnchque at MD Systems GmbH commented
Status: Needs work » Needs review
FileSize
2663756-fix.patch3.25 KB

This should fix the tests fails.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
CreditAttribution: miro_dietiker at MD Systems GmbH commented
Status: Needs review » Fixed

Thx, fixed! :-)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.