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.

Sign up for GitHub

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

Jump to bottom

Added user inside context #85

Merged
merged 4 commits into from
Sep 21, 2022
Merged

Added user inside context #85

merged 4 commits into from
Sep 21, 2022

Conversation

vitorgamer58
Copy link
Contributor

@vitorgamer58 vitorgamer58 commented Aug 25, 2022 •
edited
Loading

Fixes #84

Proposed Changes

  1. Added injection in the defaultController function so that req.user data can be accessed within usecase steps through ctx.di

Readiness Checklist

Author/Contributor

  • If documentation is needed for this change, has that been included in this pull request
  • Remember to check if code coverage decrease, if so, please implement the necessary tests

Reviewing Maintainer

  • Label as breaking if this is a large fundamental change
  • Label as either automation, bug, documentation, enhancement, infrastructure, or performance

@codecov
Copy link

codecov bot commented Aug 25, 2022 •
edited
Loading

Codecov Report

Merging #85 (833b005) into master (5582a4d) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 833b005 differs from pull request most recent head f4819ba. Consider uploading reports for the commit f4819ba to get more accurate results

@@           Coverage Diff           @@
##           master      #85   +/-   ##
=======================================
  Coverage   97.64%   97.64%           
=======================================
  Files           7        7           
  Lines         255      255           
=======================================
  Hits          249      249           
  Misses          6        6
Impacted Files Coverage Δ
src/request.js 100.00% <100.00%> (ø)
src/usecase.js 96.36% <100.00%> (+0.06%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@vitorgamer58 vitorgamer58 mentioned this pull request Aug 27, 2022
Merged
4 tasks
@dalssoft
Copy link
Member

dalssoft commented Sep 4, 2022

@vitorgamer58 great contrib!

did you used git cz to create the commit? is it marked as breaking change?

@dalssoft
Copy link
Member

dalssoft commented Sep 4, 2022 •
edited
Loading

@vitorgamer58 also, we need a PR to update the docs with info about a new variable user on the ctx.

https://herbsjs.org/docs/usecase/steps#step-context-ctx

Edit: found this PR herbsjs/herbsjs.github.io#146

This was referenced Sep 5, 2022
Closed
Closed
@vitorgamer58
Copy link
Contributor Author

@vitorgamer58 great contrib!

did you used git cz to create the commit? is it marked as breaking change?

No, I didn't use git cz, and it's not marked as breaking change.

@vitorgamer58
feat(usecase.js): added user inside context
f4819ba 
User should be acessible inside the context of usecase

BREAKING CHANGE: Projects may hbe using ctx.user wich is now user otherwise

fix herbsjs#84
@vitorgamer58
Copy link
Contributor Author

@dalssoft I did the commit using cz and market it as a breaking change

@dalssoft dalssoft merged commit 084895c into herbsjs:master Sep 21, 2022
@herbsjs-robot
Copy link

🎉 This PR is included in version 2.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@jhomarolo jhomarolo mentioned this pull request Jan 5, 2023
Open
@vitorgamer58 vitorgamer58 mentioned this pull request May 1, 2023
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

The req.user must be accessible from within the usecase steps
3 participants
@vitorgamer58 @dalssoft @herbsjs-robot