Skip to content
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

Add scopes for different conviction check states #304

Merged
merged 5 commits into from
Nov 23, 2018

Conversation

irisfaraway
Copy link
Member

@irisfaraway irisfaraway commented Nov 23, 2018

https://eaflood.atlassian.net/browse/WC-483

Now that we have workflow_states for conviction_sign_offs, we can use these to filter transient_registrations. This PR creates new scopes for the various states a conviction_sign_off can be in.

https://eaflood.atlassian.net/browse/WC-483

Now that we have workflow_states for conviction_sign_offs, we can use these to filter transient_registrations. This PR creates new scopes for the various states a conviction_sign_off can be in.
@irisfaraway irisfaraway self-assigned this Nov 23, 2018
accesslint[bot]
accesslint bot previously approved these changes Nov 23, 2018
This scope should only return transient_registrations which have a conviction_sign_off with a workflow_state of :possible_match.
This scope should only return transient_registrations which have a conviction_sign_off with a workflow_state of :checks_in_progress.
This scope should only return transient_registrations which have a conviction_sign_off with a workflow_state of :approved.
This scope should only return transient_registrations which have a conviction_sign_off with a workflow_state of :rejected.
Copy link
Member

@Cruikshanks Cruikshanks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left as a comment to see if you are ok cleaning up my code to make things consistent.

Else we could create an issue and I'll resolve it when I grab 5 mins 😁

@@ -105,4 +105,82 @@
expect(WasteCarriersEngine::TransientRegistration.pending_approval).not_to include(in_progress_renewal)
end
end

describe "conviction check scopes" do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be warned, this is essentially getting you to clean up my mess!

The way the tests are setup for the conviction check scopes differs from the way that's done in the rest of this file. I think within the same file we should aim to try and keep the pattern consistent. I'm happy to go with what you have done (what I did was based on my limited knowledge and abilities), but would you be ok as a bit of a 'boy scout exercise' to refactor the setup for the previous tests to match?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, makes sense! I'll have a bash now.

Copy link
Member Author

@irisfaraway irisfaraway Nov 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I wonder if it would make more sense to do it as a separate PR, just to keep the commit history a bit clearer? Just started it and I don't think it'll take long but it will clutter up the diff.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. That talk really sparked something! 😁

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just trying to practice what I preach in the retros 👼

New PR here: #305 I'll let you know once it's ready!

Copy link
Member

@Cruikshanks Cruikshanks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With #305 underway, happy to stamp this with :shipit: !

@irisfaraway irisfaraway merged commit 20233aa into master Nov 23, 2018
@irisfaraway irisfaraway deleted the feature/conviction-scopes branch November 23, 2018 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants