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

feat(smart-action): user can add dynamically field with hook on smart action form #717

Merged
merged 6 commits into from
Jun 7, 2021

Conversation

arnaud-moncel
Copy link
Member

@arnaud-moncel arnaud-moncel commented May 19, 2021

BREAKING CHANGE: change hook is no longer choosen by the field name, field need to have hook defined inside it definition by adding a props hook

Pull Request checklist:

  • Write an explicit title for the Pull Request, following Conventional Commits specification
  • Test manually the implemented changes
  • Review my own code (indentation, syntax, style, simplicity, readability)
  • Wonder if you can improve the existing code

@forest-bot
Copy link
Member

@arnaud-moncel arnaud-moncel changed the title feat(smart-action): user can add dynamically field with hook on smart… feat(smart-action): user can add dynamically field with hook on smart action form May 19, 2021
@arnaud-moncel arnaud-moncel force-pushed the feat/dynamic-smart-action-field branch 2 times, most recently from cd74053 to 5777599 Compare May 20, 2021 15:48
@arnaud-moncel arnaud-moncel force-pushed the chore/change-smart-action-hook-arguments branch from 1cb27ec to cc9a2a6 Compare May 25, 2021 14:19
@arnaud-moncel arnaud-moncel force-pushed the feat/dynamic-smart-action-field branch from 5777599 to c75e86e Compare May 25, 2021 14:26
@arnaud-moncel arnaud-moncel force-pushed the chore/change-smart-action-hook-arguments branch from cc9a2a6 to 3d0857b Compare June 1, 2021 09:03
@arnaud-moncel arnaud-moncel force-pushed the feat/dynamic-smart-action-field branch 2 times, most recently from 252e775 to f7e2dd1 Compare June 2, 2021 12:48
Copy link
Member

@jeffladiray jeffladiray left a comment

Choose a reason for hiding this comment

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

LGTM, apart from a few typos and tests structure.

We're almost there 🐨 🥇

try {
smartActionFieldValidator.validateSmartActionFields(action, collection.name);
} catch (error) {
logger.error(error.message);
Copy link
Member

Choose a reason for hiding this comment

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

I've distinguish on the rails side warn (For invalid property that should still work) and error for things that might be broken. Maybe we should do the same here ?

Copy link
Member Author

Choose a reason for hiding this comment

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

🤷

Copy link
Member Author

Choose a reason for hiding this comment

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

If we want to do that i will do the work inside an another PR

src/services/smart-action-field-validator.js Outdated Show resolved Hide resolved
src/services/smart-action-field-validator.js Outdated Show resolved Hide resolved
test/services/smart-action-field-validator.test.js Outdated Show resolved Hide resolved
test/services/smart-action-field-validator.test.js Outdated Show resolved Hide resolved
test/services/smart-action-field-validator.test.js Outdated Show resolved Hide resolved
test/services/smart-action-hook.test.js Outdated Show resolved Hide resolved
test/services/smart-action-hook.test.js Outdated Show resolved Hide resolved
@arnaud-moncel arnaud-moncel force-pushed the chore/change-smart-action-hook-arguments branch from 63daef2 to ce48f20 Compare June 3, 2021 13:51
@arnaud-moncel arnaud-moncel force-pushed the feat/dynamic-smart-action-field branch from d09709c to c320ef3 Compare June 3, 2021 13:52
jeffladiray
jeffladiray previously approved these changes Jun 3, 2021
@arnaud-moncel arnaud-moncel force-pushed the chore/change-smart-action-hook-arguments branch from ce48f20 to 9816936 Compare June 7, 2021 12:25
@arnaud-moncel arnaud-moncel force-pushed the feat/dynamic-smart-action-field branch from c320ef3 to 737e5d6 Compare June 7, 2021 12:26
Base automatically changed from chore/change-smart-action-hook-arguments to beta June 7, 2021 12:28
@arnaud-moncel arnaud-moncel merged commit 910df2b into beta Jun 7, 2021
@arnaud-moncel arnaud-moncel deleted the feat/dynamic-smart-action-field branch June 7, 2021 12:39
forest-bot added a commit that referenced this pull request Jun 7, 2021
# [9.0.0-beta.5](v9.0.0-beta.4...v9.0.0-beta.5) (2021-06-07)

### Features

* **smart-action:** user can add dynamically field with hook on smart action form ([#717](#717)) ([910df2b](910df2b))

### BREAKING CHANGES

* **smart-action:** change hook is no longer choosen by the field name, field need to have hook defined inside it definition by addin a props hook
@forest-bot
Copy link
Member

🎉 This PR is included in version 9.0.0-beta.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

forest-bot added a commit that referenced this pull request Jul 19, 2021
# [9.0.0](v8.7.5...v9.0.0) (2021-07-19)

### Bug Fixes

* handle composite primary key case when checking permissions for smart actions ([#756](#756)) ([30e8002](30e8002))
* **smart-action:** smart actions after trying to mitigate breaking change ([#754](#754)) ([c413992](c413992))
* remove breaking change on smart action middleware ([#739](#739)) ([b2a8f74](b2a8f74))
* update record getter usage in action route to use global scope ([#730](#730)) ([b92f158](b92f158))

### Features

* smart action hooks now have access to the http request ([#753](#753)) ([ea5cd59](ea5cd59))
* **security:** secure segments queries ([#747](#747)) ([23e8817](23e8817))
* **smart-action:** add changedField argument on hook function ([#716](#716)) ([709fe32](709fe32))
* **smart-action:** user can add dynamically field with hook on smart action form ([#717](#717)) ([910df2b](910df2b))

* feat(scopes)!: enforce scopes restrictions on a wider range of requests (#702) ([e36026e](e36026e)), closes [#702](#702)

### BREAKING CHANGES

* record is no longer send to the hook midleware & values option on smart action is no longer supported
* **smart-action:** change hook is no longer choosen by the field name, field need to have hook defined inside it definition by addin a props hook
* **smart-action:** fields parameters on hook function is no longer a map of field, it is now an array.
* the public API of the package has changed to include the parameters which are needed to evaluate scope filters. This includes PermissionMiddlewareCreator, all classes on the /exposed folder and most services.
@forest-bot
Copy link
Member

🎉 This PR is included in version 9.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

ShohanRahman pushed a commit that referenced this pull request Jan 25, 2022
Bumps [hosted-git-info](https://github.com/npm/hosted-git-info) from 2.8.5 to 2.8.9. **This update includes a security fix.**
- [Release notes](https://github.com/npm/hosted-git-info/releases)
- [Changelog](https://github.com/npm/hosted-git-info/blob/v2.8.9/CHANGELOG.md)
- [Commits](npm/hosted-git-info@v2.8.5...v2.8.9)

Signed-off-by: dependabot-preview[bot] <[email protected]>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
ShohanRahman pushed a commit that referenced this pull request Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants