add sorting to database query of repo_units

[daviian]

If sorting is wrong and UnitTypeCode is not the first one, a user can't visit to the Code Tab even with required permissions.

Furthermore fixes wrong redirect to leftmost accessible tab in repo view.

[lafriks]

LGTM

[lafriks]

Can you add integration test for this?

[daviian]

@lafriks Hopefully 😄

[daviian]

@lafriks Changed repo_unit fixtures. Wrong sorting breaks all integration tests which call /user2/repo1 without adding sorting.

[codecov-io]

Codecov Report

Merging #2603 into master will not change coverage.
The diff coverage is 50%.

@@          Coverage Diff           @@
##           master   #2603   +/-   ##
======================================
  Coverage    27.3%   27.3%           
======================================
  Files          86      86           
  Lines       17139   17139           
======================================
  Hits         4680    4680           
  Misses      11781   11781           
  Partials      678     678

Impacted Files	Coverage Δ
models/repo_unit.go	7.69% <50%> (ø)	⬆️

---

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 26e49b8...0fc1ad6. Read the comment docs.

[lunny]

But index is the column really designed for sorting.

[lafriks]

@lunny but it's not used anywhere. And current code logic depends on unit types to be in type order when checking rights and it fails if it is not so

[Morlinest]

@lafriks If you merge #2601, repo home will not fail... This PR can solve problem with showing leftmost tab.

[lafriks]

Might be that home just need to be changed so that order would not matter and fixtures moved to that PR than this one would not be needed at all

[Morlinest]

@lafriks Don't need to move fixtures there. That PR is just to fix function implementation that @daviian pointed me to while trying to solve sorting. This PR can be (at least quick) fix for redirecting to tabs in good order.

[Morlinest]

@daviian Can you use index column for sorting If we implement #2601?

[daviian]

@lunny @lafriks @Morlinest As long as index has the same sorting as the tab ordering in repo view it is fine to use index to sort types.

And since index is the integer value of the UnitType constant it is.

[lunny]

which issue will this PR fix?

[daviian]

@lunny Use the models/fixtures/repo_unit.yml I provided in this PR and run integration tests. A lot of tests will fail because of wrong sorting.

[Morlinest]

@daviian Index is not integer value of UnitType. It should be taken from Unit.Idx I think, but now it's just starting from 0 and is another bug.

Edit: Index from range is used as Index.
Edit2: Now I found places, where just int value of unit is used...

[Morlinest]

@lunny What should be source for Index? I think it should be Unit.Idx, right?

[lunny]

@Morlinest yes.

[lunny]

LGTM

[Morlinest]

@lunny Then #2621

[lafriks]

Actually I think fix should go into home handler so that it would not crash even if units come in different order

[lunny]

@lafriks Yes, it's not conflict with them. index is the sort column for design. This PR maybe not fix the crash but fixed that.

[lafriks]

Yes that's why I ask to hold on with this PR merging so that other can be fixed properly

[Morlinest]

@lafriks @lunny I think you have 1 PR for everything now :). This PR is about getting units sorted from DB, #2601 fixes home handler and #2621 fixes creating new repo units + fixes existing index data in DB.

[daviian]

@lafriks You mean that the Home handler should be capable of knowing the correct order of unit types (to open the leftmost accessible tab) without doing sorting at db layer?

[lafriks]

Yes. I think it should take unit order from models.AllUnitTypes and use it and then check if each of them is enabled

[Morlinest]

We can improve it later. And then we can just remove Index from DB as it will be useless.

[lunny]

@lafriks could this be merged?

[daviian]

Close in favour of #2621