Skip to content
This repository has been archived by the owner on Jun 5, 2024. It is now read-only.

Add require in the context #13

Closed
wants to merge 5 commits into from

Conversation

deniscoman
Copy link

Description

Add a "required_params" method to use it like this:

class FindInvoice < UseCase::Base

  required_context :current_user, :invoice_id

  def perform
    ...
  end

end

and when the perform method is called, "context_params" will create instances variables for this params.

Issue

A disscusion about this issue: #9

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 98.851% when pulling 6af5f5a on deniscoman:required_params into e8329b1 on tdantas:master.

@deniscoman
Copy link
Author

deniscoman commented Oct 2, 2017

@tdantas did you look at this?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 98.844% when pulling 3dde34f on deniscoman:required_params into e8329b1 on tdantas:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 98.883% when pulling 7d75d28 on deniscoman:required_params into e8329b1 on tdantas:master.

end

def perform(ctx = { })
tx(ExecutionOrder.run(self), ctx) do |usecase, context|
instance = usecase.new(context)
instance.tap do | me |
me.required_params
puts self

Choose a reason for hiding this comment

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

Please remove the debug statement

Copy link
Author

Choose a reason for hiding this comment

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

Done

end

def perform(ctx = { })
tx(ExecutionOrder.run(self), ctx) do |usecase, context|
instance = usecase.new(context)
instance.tap do | me |
me.required_params

Choose a reason for hiding this comment

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

This should denote an action, probably a better name would be extract_required_params.

Choose a reason for hiding this comment

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

Also, i would avoid using require (and any variation) since you don't enforce it, we are just copying the variables from the context to the usecase instance.

Copy link
Author

Choose a reason for hiding this comment

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

Now the method raises an exception when a given parameter does not exist in the context so i think required_params is a good name now.

@@ -31,6 +40,13 @@ def perform(ctx = { })

private

def concat_superclass_variable(variable)

Choose a reason for hiding this comment

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

This method does not do any actual concatenation. Please find a better name to describe the action.

Copy link
Author

@deniscoman deniscoman Oct 13, 2017

Choose a reason for hiding this comment

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

I made some changes and I deleted this method.


it 'includes params from the superclasses' do
AppUseCase = Class.new(UseCase::Base) do
required_params :param

Choose a reason for hiding this comment

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

Please rename required_params in order to denote an action.

@@ -66,6 +92,19 @@ def perform
expect(ctx.email).to eql('thiago.teixeira.dantas@gmail.com')
end

it 'receives a hash and return it' do

Choose a reason for hiding this comment

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

Remove empty line.

Copy link
Author

Choose a reason for hiding this comment

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

Done

UseCaseSubClass = Class.new(AppUseCase)

expect(UseCaseSubClass.required_params).to eql([:param])

Choose a reason for hiding this comment

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

Remove empty line

Copy link
Author

Choose a reason for hiding this comment

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

Done

@@ -40,6 +40,32 @@

end

context "required_params" do

Choose a reason for hiding this comment

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

Remove empty line

Copy link
Author

Choose a reason for hiding this comment

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

Done

@@ -85,7 +124,7 @@ def perform
pending
end

it 'fail when usecase register failure' do
it 'fail when usecase register failure' do

Choose a reason for hiding this comment

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

Remove extra space

Copy link
Author

Choose a reason for hiding this comment

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

Done. Thanks.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 98.88% when pulling 76211d1 on deniscoman:required_params into e8329b1 on tdantas:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 98.88% when pulling 7a30c74 on deniscoman:required_params into e8329b1 on tdantas:master.

Copy link

@pirvudoru pirvudoru left a comment

Choose a reason for hiding this comment

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

👍

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 98.936% when pulling c9ff87a on deniscoman:required_params into e8329b1 on tdantas:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.08%) to 98.691% when pulling 7f2df9d on deniscoman:required_params into e8329b1 on tdantas:master.

@deniscoman
Copy link
Author

@tdantas any opinion about this?

@deniscoman deniscoman closed this Jan 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants