1470532 - fixed by commit classification is buggy

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect, P1)

Description

[Andreea Pavel [:apavel]]

When trying to classify a failure with fixed by commit and adding the commit in the comment, the "fixed by commit" classification is no longer added automatically.

Also, if i only add the commit in the comment and manually select fixed by commit, the save button is grayed out and i receive " Please classify this failure before saving"

Comment 1

[Andreea Pavel [:apavel]]

As an update, checked also by selecting fixed by commit and the commit from the "Recent commits" tab, it doesn't work either.

Comment 2

[Andreea Pavel [:apavel]]

Attachment: fixed_by_commit.JPG

If we clear the classification, refresh the page,  then redo the steps (pin the failure, add fixed by commit, select the commit from recent commits) then the classification is saved, however if we move on to an intermittent failure,  after saving the fixed by commit failure, that fixed by commit remains active (see screenshot) when we pin the intermittent failure.

Comment 3

[Andreea Pavel [:apavel]]

: emorley can you please take a look?

Comment 4

[Ed Morley [:emorley]]

I can't reproduce the issue in comment 0 (could you add the steps to reproduce as a complete list of numbered steps, with all steps from start to finish, rather than as sentences/paragraphs?).

However I can reproduce the issue in comment 2.

The comment 2 issue seems pretty bad, in that it presumably will lead people to classifying later jobs as the wrong thing, because the text field is pre-filled. This along with bug 1470554 seems worthy of rolling back today's deploy - which I'll do now. (On the plus side we now have this bug and bug 1470554 filed, which we didn't as of bug 1470517 comment 1 -- hopefully we're closer to having shaken out all of the bugs ready for re-deploying next week).

Comment 5

[Ed Morley [:emorley]]

Production rollback completed a few minutes ago.

Comment 6

[Andreea Pavel [:apavel]]

(In reply to Ed Morley [:emorley] from comment #4)
> I can't reproduce the issue in comment 0 (could you add the steps to
> reproduce as a complete list of numbered steps, with all steps from start to
> finish, rather than as sentences/paragraphs?).
> 

1. Click on a failure
2. Go to the failure classification and in the comment section paste a revision number (see the failure clasification does not automatically change to fixed by commit)
3. select "fixed by commit" manually, click on save ->  "Please classify this failure before saving"
4. select "fixed by commit" and add a revision from "Recent commits", click save -> "Please classify this failure before saving"

Comment 7

[Cameron Dawson [:camd]]

Yeah, this one is pretty bad, alright.  Working on this now.

Comment 8

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3715

Comment 9

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/b89cc161e24465a1a2c3166c28103eb644a95ce2
Bug 1470532 - Fix 'fixed by commit' classification (#3715)

Comment 10

[Cameron Dawson [:camd]]

Comment on attachment 8987702 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3715

Clearing Sarah's review here since she's on vacation.

Comment 11

[Cameron Dawson [:camd]]

Andrea-- This should deploy to our staging server shortly.  Would you please test this there to verify the fix?  Thanks!!

https://treeherder.allizom.org/#/jobs?repo=mozilla-inbound

Comment 12

[Andreea Pavel [:apavel]]

Cameron: tested and everything is working properly, fixed by commit is selected automatically once you paste a revision number in the comment section. Also, the Save button is working.

Comment 13

[Andreea Pavel [:apavel]]

Cameron: apparently there is an issue where the save button is still inactive for fixed by commit
1. go to a failure, classify it as intermittent - the failure is saved
2. move on to the next failure, pin it, copy a revision and paste it in the comment section:
    Expected: the fixed by commit is automatically selected, save button is active

    Result: -  the fixed by commit is automatically selected, the save button is inactive and returns: Please classify this failure before saving

Comment 14

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3729

Comment 15

[Cameron Dawson [:camd]]

Thanks for finding that Andreea.  This latest PR should fix that.

Comment 16

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/dc96f82b9c5b965c5ec61e04b4634a4129b5a201
Bug 1470532 - Fix fixed by commit after intermittent (#3729)

Ends up I had some cruft in there from the Angular days.  Saving a
``classification`` in the state was wrong.  We store the values for
it (id and comment) in the state, then create a ``classification``
object based on those values for ``POST``ing back to the API during
``save``.

Comment 17

[Andreea Pavel [:apavel]]

(In reply to Cameron Dawson [:camd] from comment #15)
> Thanks for finding that Andreea.  This latest PR should fix that.

No problem. Checked and everything is fine now. thank you