Review Request 57: Multiple builds fixing a bug show confusing status label

[Martin Krizek]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-tflink.rhcloud.com/r/57/
-----------------------------------------------------------

Review request for blockerbugs.


Bugs: 438
    https://fedorahosted.org/fedora-qa/ticket/438


Repository: blockerbugs


Description
-------

commit 911781af9cc06e27c0a8580f07f7673c90e7329d
Author: Martin Krizek <mkrizek(a)redhat.com&gt;
Date:   Tue Dec 10 16:28:52 2013 +0100

    Sort updates by (status.desc(), pending.desc())
    
    This way an update label is correctly set when multiple updates fix a
    bug. The label is chosen by the following order (the lowest one):
    
    pending testing < testing < pending stable < stable
    
    Fixes: #438

Note: the rest of the issues with displaying label is handled outside this ticket in
https://phab.qadevel.cloud.fedoraproject.org/T26


Diffs
-----

  blockerbugs/models/update.py 89c083da5c91558d4dead118c67e4a1138b8cae1 
  blockerbugs/controllers/main.py 1795ed1d717bb85e0f3d9e7e7008d68c2aae36d8 
  blockerbugs/__init__.py dcf6c5aadb24e6d86e95d5bb3d608b81721de47f 

Diff: http://reviewboard-tflink.rhcloud.com/r/57/diff/


Testing
-------

Sorting seems to be working as expected.


Thanks,

Martin Krizek

[Martin Krizek]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-tflink.rhcloud.com/r/57/
-----------------------------------------------------------

(Updated Dec. 10, 2013, 3:54 p.m.)


Review request for blockerbugs.


Bugs: 438
    https://fedorahosted.org/fedora-qa/ticket/438


Repository: blockerbugs


Description (updated)
-------

commit 911781af9cc06e27c0a8580f07f7673c90e7329d
Author: Martin Krizek <mkrizek(a)redhat.com&gt;
Date:   Tue Dec 10 16:28:52 2013 +0100

    Sort updates by (status.desc(), pending.desc())
    
    This way an update label is correctly set when multiple updates fix a
    bug. The label is chosen by the following order (the lowest one):
    
    pending testing < testing < pending stable < stable
    
    Fixes: #438

Note: the rest of the issues with displaying label is handled outside this ticket in
https://phab.qadevel.cloud.fedoraproject.org/T21


Diffs
-----

  blockerbugs/models/update.py 89c083da5c91558d4dead118c67e4a1138b8cae1 
  blockerbugs/controllers/main.py 1795ed1d717bb85e0f3d9e7e7008d68c2aae36d8 
  blockerbugs/__init__.py dcf6c5aadb24e6d86e95d5bb3d608b81721de47f 

Diff: http://reviewboard-tflink.rhcloud.com/r/57/diff/


Testing
-------

Sorting seems to be working as expected.


Thanks,

Martin Krizek

[Martin Krizek]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-tflink.rhcloud.com/r/57/#review118
-----------------------------------------------------------


I like the concept but I disagree with your sort order change. While we _shouldn't_
have out-of-date updates displayed, it does happen and I think that we should be
prioritizing the newer update over the update that's earlier in the pipeline (ie
pending-testing over testing and testing over stable).


blockerbugs/__init__.py
<http://reviewboard-tflink.rhcloud.com/r/57/#comment146>

    Bah, have we really been running this whole time with unclosed <span> tags on
the update status labels?



blockerbugs/models/update.py
<http://reviewboard-tflink.rhcloud.com/r/57/#comment145>

    Why is this sort order change an improvement? If an older update is pending while a
newer update is stable, we'd want to show the newer update


- Tim Flink


On Dec. 10, 2013, 3:54 p.m., Martin Krizek wrote:
>  
>  -----------------------------------------------------------
>  This is an automatically generated e-mail. To reply, visit:
>  http://reviewboard-tflink.rhcloud.com/r/57/
>  -----------------------------------------------------------
>  
>  (Updated Dec. 10, 2013, 3:54 p.m.)
>  
>  
>  Review request for blockerbugs.
>  
>  
>  Bugs: 438
>      https://fedorahosted.org/fedora-qa/ticket/438
>  
>  
>  Repository: blockerbugs
>  
>  
>  Description
>  -------
>  
>  commit 911781af9cc06e27c0a8580f07f7673c90e7329d
>  Author: Martin Krizek <mkrizek(a)redhat.com&gt;
>  Date:   Tue Dec 10 16:28:52 2013 +0100
>  
>      Sort updates by (status.desc(), pending.desc())
>      
>      This way an update label is correctly set when multiple updates fix a
>      bug. The label is chosen by the following order (the lowest one):
>      
>      pending testing < testing < pending stable < stable
>      
>      Fixes: #438
>  
>  Note: the rest of the issues with displaying label is handled outside this ticket in
> https://phab.qadevel.cloud.fedoraproject.org/T21
>  
>  
>  Diffs
>  -----
>  
>    blockerbugs/models/update.py 89c083da5c91558d4dead118c67e4a1138b8cae1 
>    blockerbugs/controllers/main.py 1795ed1d717bb85e0f3d9e7e7008d68c2aae36d8 
>    blockerbugs/__init__.py dcf6c5aadb24e6d86e95d5bb3d608b81721de47f 
>  
>  Diff: http://reviewboard-tflink.rhcloud.com/r/57/diff/
>  
>  
>  Testing
>  -------
>  
>  Sorting seems to be working as expected.
>  
>  
>  Thanks,
>  
>  Martin Krizek
>  
>

[Martin Krizek]

>  On Dec. 10, 2013, 5:58 p.m., Tim Flink wrote:
>  > blockerbugs/__init__.py, line 162
>  >
> <http://reviewboard-tflink.rhcloud.com/r/57/diff/1/?file=1605#file1605line...
>  >
>  >     Bah, have we really been running this whole time with unclosed <span> tags
> on the update status labels? 
Yes :)


>  On Dec. 10, 2013, 5:58 p.m., Tim Flink wrote:
>  > blockerbugs/models/update.py, line 55
>  > <http://reviewboard-tflink.rhcloud.com/r/57/diff/1/?file=1607#file1607line...
>  >
>  >     Why is this sort order change an improvement? If an older update is pending
> while a newer update is stable, we'd want to show the newer update 
But wouldn't the older update be marked as obsolete?


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-tflink.rhcloud.com/r/57/#review118
-----------------------------------------------------------


On Dec. 10, 2013, 3:54 p.m., Martin Krizek wrote:
>  
>  -----------------------------------------------------------
>  This is an automatically generated e-mail. To reply, visit:
>  http://reviewboard-tflink.rhcloud.com/r/57/
>  -----------------------------------------------------------
>  
>  (Updated Dec. 10, 2013, 3:54 p.m.)
>  
>  
>  Review request for blockerbugs.
>  
>  
>  Bugs: 438
>      https://fedorahosted.org/fedora-qa/ticket/438
>  
>  
>  Repository: blockerbugs
>  
>  
>  Description
>  -------
>  
>  commit 911781af9cc06e27c0a8580f07f7673c90e7329d
>  Author: Martin Krizek <mkrizek(a)redhat.com&gt;
>  Date:   Tue Dec 10 16:28:52 2013 +0100
>  
>      Sort updates by (status.desc(), pending.desc())
>      
>      This way an update label is correctly set when multiple updates fix a
>      bug. The label is chosen by the following order (the lowest one):
>      
>      pending testing < testing < pending stable < stable
>      
>      Fixes: #438
>  
>  Note: the rest of the issues with displaying label is handled outside this ticket in
> https://phab.qadevel.cloud.fedoraproject.org/T21
>  
>  
>  Diffs
>  -----
>  
>    blockerbugs/models/update.py 89c083da5c91558d4dead118c67e4a1138b8cae1 
>    blockerbugs/controllers/main.py 1795ed1d717bb85e0f3d9e7e7008d68c2aae36d8 
>    blockerbugs/__init__.py dcf6c5aadb24e6d86e95d5bb3d608b81721de47f 
>  
>  Diff: http://reviewboard-tflink.rhcloud.com/r/57/diff/
>  
>  
>  Testing
>  -------
>  
>  Sorting seems to be working as expected.
>  
>  
>  Thanks,
>  
>  Martin Krizek
>  
>

[Martin Krizek]

>  On Dec. 10, 2013, 5:58 p.m., Tim Flink wrote:
>  > blockerbugs/models/update.py, line 55
>  > <http://reviewboard-tflink.rhcloud.com/r/57/diff/1/?file=1607#file1607line...
>  >
>  >     Why is this sort order change an improvement? If an older update is pending
> while a newer update is stable, we'd want to show the newer update
>  
>  Martin Krizek wrote:
>      But wouldn't the older update be marked as obsolete? 
It should be but for example, if you look at the tracking app right now, there are 2
anaconda/blivet updates - one of which has been deleted in bodhi but still shows up.

This issue was supposed to be fixed but obviously, it's not. Filed as
https://phab.qadevel.cloud.fedoraproject.org/T27

I think that the problem of phantom updates are the root cause of the problem as reported,
but I'll ask in bug to be sure.


- Tim


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-tflink.rhcloud.com/r/57/#review118
-----------------------------------------------------------


On Dec. 10, 2013, 3:54 p.m., Martin Krizek wrote:
>  
>  -----------------------------------------------------------
>  This is an automatically generated e-mail. To reply, visit:
>  http://reviewboard-tflink.rhcloud.com/r/57/
>  -----------------------------------------------------------
>  
>  (Updated Dec. 10, 2013, 3:54 p.m.)
>  
>  
>  Review request for blockerbugs.
>  
>  
>  Bugs: 438
>      https://fedorahosted.org/fedora-qa/ticket/438
>  
>  
>  Repository: blockerbugs
>  
>  
>  Description
>  -------
>  
>  commit 911781af9cc06e27c0a8580f07f7673c90e7329d
>  Author: Martin Krizek <mkrizek(a)redhat.com&gt;
>  Date:   Tue Dec 10 16:28:52 2013 +0100
>  
>      Sort updates by (status.desc(), pending.desc())
>      
>      This way an update label is correctly set when multiple updates fix a
>      bug. The label is chosen by the following order (the lowest one):
>      
>      pending testing < testing < pending stable < stable
>      
>      Fixes: #438
>  
>  Note: the rest of the issues with displaying label is handled outside this ticket in
> https://phab.qadevel.cloud.fedoraproject.org/T21
>  
>  
>  Diffs
>  -----
>  
>    blockerbugs/models/update.py 89c083da5c91558d4dead118c67e4a1138b8cae1 
>    blockerbugs/controllers/main.py 1795ed1d717bb85e0f3d9e7e7008d68c2aae36d8 
>    blockerbugs/__init__.py dcf6c5aadb24e6d86e95d5bb3d608b81721de47f 
>  
>  Diff: http://reviewboard-tflink.rhcloud.com/r/57/diff/
>  
>  
>  Testing
>  -------
>  
>  Sorting seems to be working as expected.
>  
>  
>  Thanks,
>  
>  Martin Krizek
>  
>

[Martin Krizek]

>  On Dec. 10, 2013, 5:58 p.m., Tim Flink wrote:
>  > blockerbugs/models/update.py, line 55
>  > <http://reviewboard-tflink.rhcloud.com/r/57/diff/1/?file=1607#file1607line...
>  >
>  >     Why is this sort order change an improvement? If an older update is pending
> while a newer update is stable, we'd want to show the newer update
>  
>  Martin Krizek wrote:
>      But wouldn't the older update be marked as obsolete?
>  
>  Tim Flink wrote:
>      It should be but for example, if you look at the tracking app right now, there are 2
> anaconda/blivet updates - one of which has been deleted in bodhi but still shows up.
>      
>      This issue was supposed to be fixed but obviously, it's not. Filed as
> https://phab.qadevel.cloud.fedoraproject.org/T27
>      
>      I think that the problem of phantom updates are the root cause of the problem as
> reported, but I'll ask in bug to be sure. 
After re-reading the bug, I stand corrected. That was a completely different issue than I
was thinking about in my head.

I don't like sorting on string fields by default but I also don't see much of a
way around it unless we changed the schema to use status enums instead of strings.

Go ahead and push to develop. We might want to look into changing the update status field
to an enum but that's well outside the scope of this fix.


- Tim


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-tflink.rhcloud.com/r/57/#review118
-----------------------------------------------------------


On Dec. 10, 2013, 3:54 p.m., Martin Krizek wrote:
>  
>  -----------------------------------------------------------
>  This is an automatically generated e-mail. To reply, visit:
>  http://reviewboard-tflink.rhcloud.com/r/57/
>  -----------------------------------------------------------
>  
>  (Updated Dec. 10, 2013, 3:54 p.m.)
>  
>  
>  Review request for blockerbugs.
>  
>  
>  Bugs: 438
>      https://fedorahosted.org/fedora-qa/ticket/438
>  
>  
>  Repository: blockerbugs
>  
>  
>  Description
>  -------
>  
>  commit 911781af9cc06e27c0a8580f07f7673c90e7329d
>  Author: Martin Krizek <mkrizek(a)redhat.com&gt;
>  Date:   Tue Dec 10 16:28:52 2013 +0100
>  
>      Sort updates by (status.desc(), pending.desc())
>      
>      This way an update label is correctly set when multiple updates fix a
>      bug. The label is chosen by the following order (the lowest one):
>      
>      pending testing < testing < pending stable < stable
>      
>      Fixes: #438
>  
>  Note: the rest of the issues with displaying label is handled outside this ticket in
> https://phab.qadevel.cloud.fedoraproject.org/T21
>  
>  
>  Diffs
>  -----
>  
>    blockerbugs/models/update.py 89c083da5c91558d4dead118c67e4a1138b8cae1 
>    blockerbugs/controllers/main.py 1795ed1d717bb85e0f3d9e7e7008d68c2aae36d8 
>    blockerbugs/__init__.py dcf6c5aadb24e6d86e95d5bb3d608b81721de47f 
>  
>  Diff: http://reviewboard-tflink.rhcloud.com/r/57/diff/
>  
>  
>  Testing
>  -------
>  
>  Sorting seems to be working as expected.
>  
>  
>  Thanks,
>  
>  Martin Krizek
>  
>

[Martin Krizek]

>  On Dec. 10, 2013, 5:58 p.m., Tim Flink wrote:
>  > blockerbugs/models/update.py, line 55
>  > <http://reviewboard-tflink.rhcloud.com/r/57/diff/1/?file=1607#file1607line...
>  >
>  >     Why is this sort order change an improvement? If an older update is pending
> while a newer update is stable, we'd want to show the newer update
>  
>  Martin Krizek wrote:
>      But wouldn't the older update be marked as obsolete?
>  
>  Tim Flink wrote:
>      It should be but for example, if you look at the tracking app right now, there are 2
> anaconda/blivet updates - one of which has been deleted in bodhi but still shows up.
>      
>      This issue was supposed to be fixed but obviously, it's not. Filed as
> https://phab.qadevel.cloud.fedoraproject.org/T27
>      
>      I think that the problem of phantom updates are the root cause of the problem as
> reported, but I'll ask in bug to be sure.
>  
>  Tim Flink wrote:
>      After re-reading the bug, I stand corrected. That was a completely different issue
> than I was thinking about in my head.
>      
>      I don't like sorting on string fields by default but I also don't see much of
> a way around it unless we changed the schema to use status enums instead of strings.
>      
>      Go ahead and push to develop. We might want to look into changing the update status
> field to an enum but that's well outside the scope of this fix. 
I know. This is just a part of the fix, the rest will be in
https://phab.qadevel.cloud.fedoraproject.org/T21 as I mentioned in the fix description.
Should have probably done it in one review, sorry for confusion.


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-tflink.rhcloud.com/r/57/#review118
-----------------------------------------------------------


On Dec. 10, 2013, 3:54 p.m., Martin Krizek wrote:
>  
>  -----------------------------------------------------------
>  This is an automatically generated e-mail. To reply, visit:
>  http://reviewboard-tflink.rhcloud.com/r/57/
>  -----------------------------------------------------------
>  
>  (Updated Dec. 10, 2013, 3:54 p.m.)
>  
>  
>  Review request for blockerbugs.
>  
>  
>  Bugs: 438
>      https://fedorahosted.org/fedora-qa/ticket/438
>  
>  
>  Repository: blockerbugs
>  
>  
>  Description
>  -------
>  
>  commit 911781af9cc06e27c0a8580f07f7673c90e7329d
>  Author: Martin Krizek <mkrizek(a)redhat.com&gt;
>  Date:   Tue Dec 10 16:28:52 2013 +0100
>  
>      Sort updates by (status.desc(), pending.desc())
>      
>      This way an update label is correctly set when multiple updates fix a
>      bug. The label is chosen by the following order (the lowest one):
>      
>      pending testing < testing < pending stable < stable
>      
>      Fixes: #438
>  
>  Note: the rest of the issues with displaying label is handled outside this ticket in
> https://phab.qadevel.cloud.fedoraproject.org/T21
>  
>  
>  Diffs
>  -----
>  
>    blockerbugs/models/update.py 89c083da5c91558d4dead118c67e4a1138b8cae1 
>    blockerbugs/controllers/main.py 1795ed1d717bb85e0f3d9e7e7008d68c2aae36d8 
>    blockerbugs/__init__.py dcf6c5aadb24e6d86e95d5bb3d608b81721de47f 
>  
>  Diff: http://reviewboard-tflink.rhcloud.com/r/57/diff/
>  
>  
>  Testing
>  -------
>  
>  Sorting seems to be working as expected.
>  
>  
>  Thanks,
>  
>  Martin Krizek
>  
>

[Martin Krizek]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-tflink.rhcloud.com/r/57/
-----------------------------------------------------------

(Updated Dec. 16, 2013, 1:24 p.m.)


Status
------

This change has been marked as submitted.


Review request for blockerbugs.


Bugs: 438
    https://fedorahosted.org/fedora-qa/ticket/438


Repository: blockerbugs


Description
-------

commit 911781af9cc06e27c0a8580f07f7673c90e7329d
Author: Martin Krizek <mkrizek(a)redhat.com&gt;
Date:   Tue Dec 10 16:28:52 2013 +0100

    Sort updates by (status.desc(), pending.desc())
    
    This way an update label is correctly set when multiple updates fix a
    bug. The label is chosen by the following order (the lowest one):
    
    pending testing < testing < pending stable < stable
    
    Fixes: #438

Note: the rest of the issues with displaying label is handled outside this ticket in
https://phab.qadevel.cloud.fedoraproject.org/T21


Diffs
-----

  blockerbugs/models/update.py 89c083da5c91558d4dead118c67e4a1138b8cae1 
  blockerbugs/controllers/main.py 1795ed1d717bb85e0f3d9e7e7008d68c2aae36d8 
  blockerbugs/__init__.py dcf6c5aadb24e6d86e95d5bb3d608b81721de47f 

Diff: http://reviewboard-tflink.rhcloud.com/r/57/diff/


Testing
-------

Sorting seems to be working as expected.


Thanks,

Martin Krizek