Adjust milestone sort order for due date. Non due date milestone will be considered after all due date milestone

[lunny]

Fix #35194

Related to #32339

[wxiaoguang]

I don't think the code reads right. The deadline_unix is null-able in database IIRC.

[lunny]

I don't think the code reads right. The deadline_unix is null-able in database IIRC.

1bafbb6

[6543]

looks clustered, but did not come up with something better jet :/

[wxiaoguang]

Related to #32339

Well, I regretted that I approved 32339 ..... and I think we shouldn't add more patches.

If you search "deadline_unix" in the code base, you can see there are more places using it for comparing and sorting.

So the correct fix should be introducing a very large magic unix timestamp for "no due date", but never use it as a real date/time struct

[6543]

I think magic values are not good and should mostly be avoided ...

... new maintainers just get confused ...
... so the tend to be hatder to maintain
... as the name already implyes with magic, the meaning is not clear at first sight and you have to dig deep into why what appens for each of such values
... that should be documented, but we know about docs being ... just what they are now :/
... if set you need to migrate stuf if you need to change it

but it would make the db query shorter yes ... :/

🤔

[wxiaoguang]

I don't like magic numbers and seldom use them.

But for this case, if you can make sure:

• all "deadline_unix" related SQLs are written correctly (fix all other places)
• complex SQLs should be covered by tests
• future developers won't make such low-level mistakes (help them to fix all "order by deadline_unix" and "deadline_unix < ?" bugs)

Then just do it.

[wxiaoguang]

But for this case, if you can make sure:

* all "deadline_unix" related SQLs are written correctly (fix all other places)
* complex SQLs should be covered by tests
* future developers won't make such low-level mistakes (help them to fix all "order by deadline_unix" and "deadline_unix < ?" bugs)

Then just do it.

@6543 will you do it?

[6543]

first yes i can look into the code if you want

tests should have exist in the first place for new features but i get it ... efen i need to be rememberd to add them and it cam delay features etc ... but as maintainers we should ask for them, we have an extensive test

when reviewing one should always look for such things ...
... if it is so dam hard we should encapsulate it away behind func we control ... and dont allow sql in various places ... -> proxy pattern

[wxiaoguang]

first yes i can look into the code if you want

The bug should be fixed.