Add a quick check to ensure old version is older than the new version

[willdurand]

Fixes #365

---

This will be useful to make sure both #111 and #314 behave correctly. All the other components in Compare won't have to deal with the problem of knowing which version is the "old" one and which version is the "new" one. This check happens before any reviewer API calls.

It works because version IDs are integers and newer version IDs get higher numbers (there is some sort of auto-increment). I know we should avoid relying on the auto-increment property of an ID in general, but this is a very simple solution that works well for our purpose. I don't think we'll change the type of version IDs anytime soon (we'll likely always deal with integers) and I don't think we'll change the behavior that currently guarantees that new version IDs have higher numbers than older version IDs. Hence the proposed patch.

The longer version of this (mvp-polish, maybe?) would be to:

• fetch both versions separately (I don't think we need version data in the compare page so it would mean fetching the data only for the comparison)
• compare both versions, but based on what? There is currently no reliable value exposed to perform such a comparison. A created_at date could work, if this is something that we store in the database already
• do what's in this patch

If you browse: http://localhost:3000/en-US/compare/502955/versions/1541799...1541798/, this patch detects that the right version is lower than the left version and inverts them + update the URL.

Let me know what you think!

[codecov-io]

Codecov Report

Merging #342 into master will increase coverage by 0.17%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #342      +/-   ##
==========================================
+ Coverage   96.56%   96.74%   +0.17%     
==========================================
  Files          34       34              
  Lines         553      583      +30     
  Branches      120      127       +7     
==========================================
+ Hits          534      564      +30     
  Misses         18       18              
  Partials        1        1

Impacted Files	Coverage Δ
src/pages/Compare/index.tsx	100% <100%> (ø)	⬆️
src/components/DiffView/index.tsx	100% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1c2910b...32b1185. Read the comment docs.

[kumar303]

I'd like to see the component always load versions from the URL

[kumar303]

r+wc, just needs test cleanup

The new loading logic also makes me cry but the last approach was too confusing, even if it had a small scope. It went against the main principle of React, props->render, which I think could lead to problems. Let's improve the code later with useEffect #363