search

guardian / typerighter

Even if you’re the right typer, couldn’t hurt to use Typerighter!
Apache License 2.0
276 stars 12 forks source link

Sort by rule order #381

Closed samanthagottlieb closed 1 year ago

samanthagottlieb commented 1 year ago

What does this change?

This adds a rule_order column to the rules_draft table, which will be used to determine prioritisation of rules that may conflict.

Previously (before the TR manager), rule order was determined by the order of the rule in the spreadsheet. This order has been maintained in the database by the rule id - rule_order in the rules_live table was determined by copying this id when a rule is published.

When there is already data in the database, the logic added in this PR copies the id in rules_draft into a new rule_order column (done as an evolution). When a new rule is added, a query will find the latest rule order and increment this by 1. If there is no data in the database, the rule_order will be assigned according to the spreadsheet row index, thus maintaining the spreadsheet order.

Trello card is here.

Next

Sort by rule order on the client. In future we'll look at implementing a way for users to rearrange the order of conflicting rules.

How to test

This can be tested locally by running sbt, and running the tests with:

project ruleManager
test

All tests should pass and the latest evolution should be applied. If your database already contained data, all rows in the rules_draft column should be populated, with the values of rules_draft matching the values of the rule's id.

To test the logic when the database starts without any tables, drop your local database tables, run the manager app and press the big red Destroy all rules in the manager... button in the UI. The rules_draft table should be created with a rule_order column. The value of rule_order won't necessarily match the id as the id auto increment counter does not reset automatically.

This has been tested on CODE. Instructions here can be followed to connect to the CODE database.

samanthagottlieb commented 1 year ago

Can see this running in CODE, looks good! Top work. One non-blocking comment.

A quick note that the title of this PR isn't quite accurate as I understand it – we're adding a rule order column to the draft rules table, but not sorting by it yet!

I think that would be good to do – all we'd need is an order by clause for our find methods in DbRuleDraft. Happy for us to do this in this PR, or a separate PR.

Good point on the PR title! I've added an ORDER BY clause to the find methods. In terms of testing this (or testing any query using the ScalaJDBC lib) I ran the app locally, played around in the Typerighter UI, looked at the Docker logs, copied the latest query, and ran this in my IntelliJ query console to check the results were ordered by rule_order. Is this the best way to go about it, or is there a more efficient way?

jonathonherbert commented 1 year ago

@samanthagottlieb that sounds great. It's only a small change, what you did would have worked well (guessing the presence of the ORDER BY clause was enough to convince you it was doing the right thing). Quickest feedback loop I'd reach for to check this is probably to write a test with a couple of rules having differing rule_order values, and using ~testOnly to iterate.

samanthagottlieb commented 1 year ago

@samanthagottlieb that sounds great. It's only a small change, what you did would have worked well (guessing the presence of the ORDER BY clause was enough to convince you it was doing the right thing). Quickest feedback loop I'd reach for to check this is probably to write a test with a couple of rules having differing rule_order values, and using ~testOnly to iterate.

Thanks for this Jon, perhaps I merged prematurely and should have added a test or two!