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

Include context inside uc.authorize() #44

Open
thgpdr opened this issue Jul 28, 2021 · 3 comments
Open

Include context inside uc.authorize() #44

thgpdr opened this issue Jul 28, 2021 · 3 comments
Labels
hacktoberfest help wanted Extra attention is needed question Further information is requested severity-minor Item is not urgent

Comments

@thgpdr
Copy link
Contributor

thgpdr commented Jul 28, 2021

Hey guys, I came across a problem theses days, which was:
I needed to test a use case that had an external client call inside authorize(), like calling another micro service to check some user permissions before authorizing the use case execution.

I did realize that I could not inject the external client while testing, because .authorize() do not receive the use case context as a parameter, so I needed to mock the hole external client require or override the ._authorize()

const ucMyBusinessRule = MyBusinessRule()
ucMyBusinessRule._authorize = async () => {
  return { isOk: true }
}

await ucMyBusinessRule.authorize()
const ret = await ucMyBusinessRule.run(parameters)

if (ret.isErr) console.log(ret.err)

const { ruleRet } = ret.ok

expect(ruleRet.isValid()).to.be.true

I dont know if its an real issue and we should add ctx inside the authorize() or that is the expected behavior, since its a convention to not use context inside authorize step. What your thoughts on this?

@thgpdr thgpdr added the question Further information is requested label Jul 28, 2021
@PedroHaupenthal
Copy link
Contributor

@thgpdr Complement in your comment, I went through a very similar situation, that my authorize() called an injected external client but in the unit test the ctx always returned empty.

I found this to be because setup generates the context only after usecase.run().
But authorize() is done before run().

Take a look at how usecases are written in the new herbs-cli . You will see that setup is not used and I believe that this can solve the problem.

@dalssoft
Copy link
Member

@PedroHaupenthal the way cli handles dependencies/injection is under discussion, so I would not rely on then.

@thgpdr it would be nice to make a test:
(1) change the order and call this._setup(this._mainStep.context) on async authorize(user) {
(2) check if it's ok to access the context using this.ctx within the authorize()

@jhomarolo jhomarolo added help wanted Extra attention is needed severity-minor Item is not urgent labels Dec 24, 2021
@dalssoft dalssoft moved this to More discussion is needed in Herbs - 3rd Anniversary Edition May 25, 2022
@jhomarolo
Copy link
Contributor

Is this issue still valid after #85 is finished?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest help wanted Extra attention is needed question Further information is requested severity-minor Item is not urgent
Projects
Status: More discussion is needed
Development

No branches or pull requests

5 participants