Look at the current reviewRef when submitting

[iveqy]

When a review has been rebased and then force pushed to update a review,
for example to fix review comments. It can become impossible to submit
with the error message:

Refusing to submit a non-fast-forward review. First merge the target
ref.

this even if the review ref is a ancestor of the target ref and the
merge should been a fast forward merge. This is because the check for
appraise submit is done against the first commit sha1 that was used and
not for the actually reviewRef that will be merged. This fixes that.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[iveqy]

For some reason this mess up the git-appraise list view where submitted code reviews will still be shown as open. IT tells me that there's more to this than just this merge request.

[ojarjur]

Hi @iveqy, thanks for taking the time and putting in the effort to contribute to the tool.

As you noted, there is more to what is going on than just the IsAncestor check.

When you rebase the commits in a review outside of using the git appraise rebase command, then we lose any link between the original commit and the commit you are trying to submit.

That means that even if we disabled the IsAncestor check, the original review would not show up as being submitted, because git-appraise does not know the commits are related.

This can be fixed by running git appraise rebase prior to running git appraise submit, so maybe instead all we need to do is catch this scenario and give the user appropriate instructions.

I'll add an inline comment giving an example of what I am thinking.

[ojarjur]

We still want to use the result of GetHeadCommit in the below check. It will actually use the review ref head if the metadata linking the review commit to the review ref head is properly set up.

However, we also want to give the user instructions on how to fix it if that metadata is not correct.

I think a better fix might be to add logic right after this (right before we check the ancestry of the target ref) to double check that the metadata is correct and give the user instructions on how to fix it if it is not.

Something like:

	isReviewAssociatedWithRef, err := repo.IsAncestor(source, r.Request.ReviewRef)
	if err != nil {
		return err
	}
	if !isReviewAssociatedWithRef {
		return fmt.Errorf("The review commit is not associated with the review ref %q. If the change has been rebased, then reassociate the review commit with the review ref using the command `git appraise rebase %s`", r.Request.ReviewRef, r.Revision)
	}