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

fix(charts): user with permissions level that allows charts creation or edition should always be allow to perform charts requests #60

Merged
merged 4 commits into from
Sep 6, 2022

Conversation

matthv
Copy link
Member

@matthv matthv commented Sep 5, 2022

Definition of Done

General

  • Write an explicit title for the Pull Request, following Conventional Commits specification
  • Test manually the implemented changes
  • Validate the code quality (indentation, syntax, style, simplicity, readability)

Security

  • Consider the security impact of the changes made

@matthv matthv changed the title Fix/update permission chart fix(charts): user with permissions level that allows charts creation or edition should always be allow to perform charts requests Sep 5, 2022
@matthv matthv marked this pull request as ready for review September 5, 2022 15:19
@codeclimate
Copy link

codeclimate bot commented Sep 5, 2022

Code Climate has analyzed commit f9ee038 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (99% is the threshold).

This pull request will bring the total coverage in the repository to 100.0%.

View more on Code Climate.

@matthv matthv requested a review from Thenkei September 5, 2022 15:22
@ForestAdmin ForestAdmin deleted a comment from forest-bot Sep 6, 2022
@forest-bot
Copy link
Member

Copy link

@Thenkei Thenkei left a comment

Choose a reason for hiding this comment

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

  • ✅ Code review - You nailed it! 🚀 I just have one question.
  • ✅ Automatic tests
    • ✅ Unit tests
  • 🕐 Manual tests - We can make a call to see this together in live? 🤩
  • ✅ PR title

Comment on lines +73 to +81
'id' => $this->data['id'],
'email' => $this->data['email'],
'first_name' => $this->data['first_name'],
'last_name' => $this->data['last_name'],
'team' => $this->data['teams'][0],
'tags' => $this->data['tags'],
'rendering_id' => $this->renderingId,
'exp' => $this->expirationInSeconds(),
'permission_level' => $this->data['permission_level'],
Copy link

Choose a reason for hiding this comment

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

🚀

@@ -10,7 +10,6 @@
use Illuminate\Http\Request;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Facades\Gate;
use Illuminate\Support\Str;
Copy link

Choose a reason for hiding this comment

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

❔ Is this related to this topic?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope TBH, I saw that this import was not used by looking at the authorization provider.
This file is used to add Policies for the charts => https://github.com/ForestAdmin/laravel-forestadmin/blob/main/src/Providers/AuthorizationProvider.php#L44-L56

@matthv matthv merged commit b467b35 into main Sep 6, 2022
@matthv matthv deleted the fix/update-permission-chart branch September 6, 2022 13:01
forest-bot added a commit that referenced this pull request Sep 6, 2022
## [1.1.3](v1.1.2...v1.1.3) (2022-09-06)

### Bug Fixes

* **charts:** user with permissions level that allows charts creation or edition should always be allow to perform charts requests ([#60](#60)) ([b467b35](b467b35))
@forest-bot
Copy link
Member

🎉 This PR is included in version 1.1.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

arnaud-moncel pushed a commit that referenced this pull request Jul 17, 2023
…or edition should always be allow to perform charts requests (#60)
arnaud-moncel pushed a commit that referenced this pull request Jul 17, 2023
## [1.1.3](v1.1.2...v1.1.3) (2022-09-06)

### Bug Fixes

* **charts:** user with permissions level that allows charts creation or edition should always be allow to perform charts requests ([#60](#60)) ([b467b35](b467b35))
forest-bot added a commit that referenced this pull request Jul 17, 2023
# [1.0.0-beta.24](v1.0.0-beta.23...v1.0.0-beta.24) (2023-07-17)

### Bug Fixes

* **charts:** user with permissions level that allows charts creation or edition should always be allow to perform charts requests ([#60](#60)) ([e33a601](e33a601))
* **query-builder:** patch eagerload relationships on collection ([#66](#66)) ([0c1ef79](0c1ef79))
* **relationship:** ignore methods with parameters that return relationships  ([#68](#68)) ([99d380a](99d380a))
* **security:** update guzzle version (with dependency) to 7.4.5 ([#58](#58)) ([3b26ebd](3b26ebd))
* **security:** update guzzle version to 7.4.3 ([#52](#52)) ([b32b9db](b32b9db))
* **smart-collection:** add support of smart-relationship ([#63](#63)) ([7b69434](7b69434))
* add database type timestamp for schema generation ([#57](#57)) ([087eaea](087eaea))
* permission to avoid any conflict with laravel policy ([#62](#62)) ([883129a](883129a))
* render jsonapi when a model uses eager load relation ([#61](#61)) ([656500c](656500c))

### Features

* add support for multiple models directories ([#56](#56)) ([bde2f5d](bde2f5d))
* allow to use agent with laravel 10 ([#69](#69)) ([69b01cc](69b01cc))
* allow user to onboard to the new eloquent datasource from forestadmin/agent-php ([#72](#72)) ([09f4822](09f4822))
* **auth:** remove callbackUrl parameter on authentication ([#59](#59)) ([647536e](647536e))

### BREAKING CHANGES

* allow user to onboard to the new eloquent datasource from forestadmin/agent-php
arnaud-moncel pushed a commit that referenced this pull request Jul 17, 2023
…or edition should always be allow to perform charts requests (#60)
arnaud-moncel pushed a commit that referenced this pull request Jul 17, 2023
## [1.1.3](v1.1.2...v1.1.3) (2022-09-06)

### Bug Fixes

* **charts:** user with permissions level that allows charts creation or edition should always be allow to perform charts requests ([#60](#60)) ([b467b35](b467b35))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants