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(schema): developers can specify manually the path of the .forestadmin-schema.json file #698

Merged
merged 11 commits into from
May 5, 2021

Conversation

arnaudbesnier
Copy link
Contributor

@arnaudbesnier arnaudbesnier commented Apr 26, 2021

Inspired by this Pull Request #488
And, thus, co-authored with @julienfouilhe

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

@arnaudbesnier arnaudbesnier changed the title feat(schema): developers can specify manually the path of the .forest… feat(schema): developers can specify manually the path of the .forestadmin-schema.json file Apr 26, 2021
@arnaudbesnier arnaudbesnier force-pushed the feat/support-custom-schema-path branch from f3ae5a1 to c4bfcaa Compare April 26, 2021 14:35
Base automatically changed from feat/support-yarn-2-plug-n-play to master April 27, 2021 11:53
@arnaudbesnier arnaudbesnier force-pushed the feat/support-custom-schema-path branch from 2915ada to 6497ec3 Compare April 27, 2021 15:16
@arnaudbesnier arnaudbesnier force-pushed the feat/support-custom-schema-path branch from 6497ec3 to b14ced7 Compare April 27, 2021 15:33
@@ -0,0 +1,47 @@
class ProjectDirectoryFinder {
Copy link
Contributor Author

@arnaudbesnier arnaudbesnier May 4, 2021

Choose a reason for hiding this comment

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

Same code as the util ProjectDirectoryUtils(deleted below) with:

  • "full" dependency injection (path)
  • pathSchemaDir option to configure a custom schema directory

Copy link
Member

@arnaud-moncel arnaud-moncel left a comment

Choose a reason for hiding this comment

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

I did not review the test for now, I just want to see if my suggestion make sens for you.
By this suggestion below I want to homogenize the existing code inside the ConfigStorclass.
My major argument is to use the path we tested.
In your code you test if this this.path.resolve('.', this.lianaOptions.schemaDir) path exist and you use this projectDirectoryFinder.getAbsolutePath() computed by another way.
I don't know why but I doesn't have a good feeling with this.
Let me know 😀

@codeclimate
Copy link

codeclimate bot commented May 5, 2021

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

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

This pull request will bring the total coverage in the repository to 55.0% (0.1% change).

View more on Code Climate.

Copy link
Member

@arnaud-moncel arnaud-moncel left a comment

Choose a reason for hiding this comment

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

Nice work 🎉💪

@arnaudbesnier arnaudbesnier merged commit c27bfb9 into master May 5, 2021
@arnaudbesnier arnaudbesnier deleted the feat/support-custom-schema-path branch May 5, 2021 08:52
forest-bot added a commit that referenced this pull request May 5, 2021
# [8.5.0](v8.4.0...v8.5.0) (2021-05-05)

### Features

* **schema:** developers can specify manually the path of the .forestadmin-schema.json file ([#698](#698)) ([c27bfb9](c27bfb9))
@forest-bot
Copy link
Member

🎉 This PR is included in version 8.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

felipe-augusto added a commit to felipe-augusto/forest-express-sequelize that referenced this pull request Oct 14, 2021
ShohanRahman pushed a commit that referenced this pull request Jan 25, 2022
ShohanRahman pushed a commit that referenced this pull request Jan 25, 2022
# [8.0.0-beta.3](ForestAdmin/forest-express-sequelize@v8.0.0-beta.2...v8.0.0-beta.3) (2021-05-26)

### Bug Fixes

* **smart-actions-change-hook:** record is no longer altered and is sent correctly ([#728](ForestAdmin/forest-express-sequelize#728)) ([2ac7af8](ForestAdmin/forest-express-sequelize@2ac7af8))
* distribution charts using groupby on a relationship throws 403 Forbidden ([#725](ForestAdmin/forest-express-sequelize#725)) ([30e6744](ForestAdmin/forest-express-sequelize@30e6744))
* regression when fetching has-many and not selecting any fields on a hasone/belongsto relation ([#720](ForestAdmin/forest-express-sequelize#720)) ([74ed623](ForestAdmin/forest-express-sequelize@74ed623))
* **schema:** do not remove primary key fields from the schema when tables have foreign keys that are primary keys ([8844fb5](ForestAdmin/forest-express-sequelize@8844fb5))
* **search:** enable to search for a big integer in an ID field ([#695](ForestAdmin/forest-express-sequelize#695)) ([9f8132c](ForestAdmin/forest-express-sequelize@9f8132c))
* **search:** searching for a big int value should not break if there is a regular integer field ([#694](ForestAdmin/forest-express-sequelize#694)) ([af076ad](ForestAdmin/forest-express-sequelize@af076ad))
* **security:** patch ssri dependency vulnerability ([#690](ForestAdmin/forest-express-sequelize#690)) ([6b0770d](ForestAdmin/forest-express-sequelize@6b0770d))

### Features

* **browsing-context:** allow `Forest-Context-Url` header to give the current browser url of users ([#665](ForestAdmin/forest-express-sequelize#665)) ([c46fd66](ForestAdmin/forest-express-sequelize@c46fd66))
* **filters:** add support for the \`model.field IN array\` filter condition ([#719](ForestAdmin/forest-express-sequelize#719)) ([5f58457](ForestAdmin/forest-express-sequelize@5f58457))
* add support for belongsTo and hasOne filters on related data ([#715](ForestAdmin/forest-express-sequelize#715)) ([2bc769e](ForestAdmin/forest-express-sequelize@2bc769e))
* support yarn 2 plug n play install mode ([#698](ForestAdmin/forest-express-sequelize#698)) ([64b5734](ForestAdmin/forest-express-sequelize@64b5734))

### BREAKING CHANGES

* **browsing-context:** users willing to use this header needs either to clean the allowed headers or to add this specific header in the CORS configuration
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