Editor: Allow null values for foreign ref fields or required fields where permitted #133
Conversation
…mpty This fixes a bug where create/update requests with foreign refs (such as route_id in fare_rules) could not be null even if the table definition permits the null value. fixes catalogueglobal/datatools-ui#224
Codecov Report
@@ Coverage Diff @@
## dev #133 +/- ##
===========================================
+ Coverage 53.7% 55.17% +1.46%
- Complexity 697 731 +34
===========================================
Files 143 143
Lines 7170 7176 +6
Branches 828 830 +2
===========================================
+ Hits 3851 3959 +108
+ Misses 2996 2880 -116
- Partials 323 337 +14
Continue to review full report at Codecov.
|
|
I'll assign this back to you, Landon, and let you decide if this is a good time to start adding tests. |
fixes catalogueglobal/datatools-ui/#237
landonreed
changed the title
|
OK, @evansiroky. I added one other bug fix to this PR in addition to a new CRUD test for fares. |
| true | ||
| ); | ||
| LOG.info("delete {} output:", fareTable.name); | ||
| LOG.info(updateOutput); |
There was a problem hiding this comment.
This should probably be deleteOutput.
| ); | ||
| LOG.info(fareRulesSql); | ||
| ResultSet fareRulesResultSet = testDataSource.getConnection().prepareStatement(fareRulesSql).executeQuery(); | ||
| assertThat(fareRulesResultSet.getFetchSize(), equalTo(0)); |
There was a problem hiding this comment.
Now that I look at these tests, it seems like they could really use a sql check to make sure that the data was created and updated. Also, these 2 lines could be made into a reusable method assertThatSqlQueryYieldsZeroRows or something like that.
There was a problem hiding this comment.
Hmm, actually I think the checking for created/updated data gets a little muddied because we would then need to ensure that we're using the same connection to see the data we expect. I think the first part of your comment is out of scope, but I will add a TODO.
|
Excellent! I gave an idea of how to improve the CRUD tests, but if that seems out of scope for this PR, let's create an issue for that and do that later. |
|
🎉 This PR is included in version 4.1.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This fixes a couple of editor bugs:
fare_rules#route_id)could not be null even if the table definition permits the null value (Can't create a fare rule using zones catalogueglobal/datatools-ui#224) and
fare_attributes#transfers) were not allowed in create/update requests (Can't create a fare with unlimited transfers catalogueglobal/datatools-ui#237)It also adds a basic CRUD test for fares.