-
-
Notifications
You must be signed in to change notification settings - Fork 198
Member search #2260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Member search #2260
Conversation
856ab89
to
2584ae3
Compare
2584ae3
to
c11f338
Compare
89256ab
to
c44a3ff
Compare
@matyikriszta do you mind giving me a sense-check on this? Does it look like it's going in the right direction? |
13e5ec6
to
28cf0f8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wanted to share some Rails thoughts.
EDIT: I don't want to block you, perhaps we can pair sometime, to brain-sync on this!
28cf0f8
to
349108d
Compare
what happens if you try to search with no search terms? |
df5f60f
to
cc279c5
Compare
That was a good question. I've added a check against it returning the entire table! |
Tests are not failing, coveralls is 😞 |
d98221a
to
534587f
Compare
@olleolleolle @matyikriszta @KimberleyCook - I'd like to integrate this code into the codebase before I start working on calling it from elsewhere. I suspect the approach is going to be a little complex, and I don't want to make a massive change all at once. Tests are passing locally but not here - there's more flakiness around I think. Going forward, I'm thinking about how this can be incorporated into other calling services. I'm considering moving the logic in |
This sets up a basic test case, controller, and a form for doing search. This will allow a calling service to find a member and then do something with them
This has been educational! I've added four tests to check for how the controller processes the member search journey. I'm happy with where it's got to, and it's meant I've had a chance to clarify my controller code.
This is a small change that lets me access the db from my IDE. In turn this let me generate a diagram of the database structure. It took several minutes
These tests will track the full journey for an admin user who wants to add a Member to a workshop
This feature test checks the flow when there's only one matching member Signed-off-by: jonathan.kerr <[email protected]>
This adds three tests for the model method and improves the model method Signed-off-by: jonathan.kerr <[email protected]>
Signed-off-by: jonathan.kerr <[email protected]>
Based on feedback from @olleolleolle, I've updated this controller to only use local variables Signed-off-by: jonathan.kerr <[email protected]>
Signed-off-by: jonathan.kerr <[email protected]>
I want to avoid returning the whole Member table if a user accidentally forgets to put in a search term. Although this can be managed with validation, I'm applying this at the lowest level for maximal protection Signed-off-by: jonathan.kerr <[email protected]>
Signed-off-by: jonathan.kerr <[email protected]>
This ensures there will always be at least two Members in the tests where they're needed Signed-off-by: jonathan.kerr <[email protected]>
This test essentially just replicates the test above
28ee853
to
646ee02
Compare
This adds a controller and little view for doing member searches. Full implementation of this pattern will require calling services to add a
callback
param to the request toMemberSearch
, which it will thenGET
once the Member(s) have been found.If only one member is found from the search, I've chosen to assume that it's correct and we skip straight to redirecting to the calling service. That might be a faulty assumption.
I'm going to clear up the commit history, but it'll give you an early sense of how this is working. Screenshots!