refactor(x/data): remove unique constraint on resolver url #1128
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1128 +/- ##
=======================================
Coverage 69.31% 69.32%
=======================================
Files 216 216
Lines 21948 21908 -40
=======================================
- Hits 15214 15187 -27
+ Misses 5407 5389 -18
- Partials 1327 1332 +5
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
🙏 thank u @ryanchristo !
Just dropped at note in #1111 (comment) echo'ing that I think querying for resolvers by URL with pagination (returning all resolvers with their ID & manager) would be useful. Potentially in a follow-up.
|
|
||
| Rule: the resolver is defined if the resolver url is unique | ||
| Scenario: the url is unique |
There was a problem hiding this comment.
| Scenario: the url is unique | |
| Scenario: a resolver for a new url is registered |
There was a problem hiding this comment.
In general, I was thinking the scenario would be short and to the point and the rule would be more descriptive. Given the rule is "the resolver is defined with no unique constraints on the URL", the two scenarios are "the url is unique" and "the url is not unique". This is also how the BDD books write scenarios. For example,
Rule: Any visitor to the website can place a customer-collection order
Scenario: Unauthenticated customer chooses to collect order
Given the customer is unauthenticated
When they choose to collect their order
Then they should be asked to supply contact details
Scenario: Authenticated customer chooses to collect order
Given the customer is authenticated
When they choose to collect their order
Then they should be asked to confirm contact details
There was a problem hiding this comment.
This also produces the test logs:
=== RUN TestDefineResolver
=== RUN TestDefineResolver/Define_Resolver
=== RUN TestDefineResolver/Define_Resolver/a_resolver_for_a_new_url_is_registered
=== RUN TestDefineResolver/Define_Resolver/a_resolver_is_registered_with_a_url_that_is_already_registered
Versus the current test logs:
=== RUN TestDefineResolver
=== RUN TestDefineResolver/Define_Resolver
=== RUN TestDefineResolver/Define_Resolver/the_url_is_unique
=== RUN TestDefineResolver/Define_Resolver/the_url_is_not_unique
There was a problem hiding this comment.
We are also "defining" a resolver here, not "registering" data to the resolver.
| Given alice has defined the resolver | ||
| When alice attempts to define the resolver | ||
| Then expect the error "resolver URL already exists" | ||
| Scenario: the url is not unique |
There was a problem hiding this comment.
| Scenario: the url is not unique | |
| Scenario: a resolver is registered with a url that is already registered |
|
@clevinson Would be happy to further discuss best format for scenarios and update throughout but the current format is aligned with how I've been writing scenarios elsewhere and I think that if we include a descriptive rule, scenarios should be concise and provide only the necessary information, which makes reading through the test logs easier when debugging failing scenarios. |
Description
Closes: #1111
This pull request removes the unique constraint on url and updates query resolver to take the resolver id rather than url.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking change