Skip to content
This repository has been archived by the owner on Mar 21, 2021. It is now read-only.

New unary option readOnly to declare an entity as read-only #370

Merged
merged 2 commits into from
Sep 22, 2019
Merged

New unary option readOnly to declare an entity as read-only #370

merged 2 commits into from
Sep 22, 2019

Conversation

murdos
Copy link
Contributor

@murdos murdos commented Sep 20, 2019

Fixes #318

I used the keywork readOnly, but only at end I realized that it could have been readonly (both are used in #318).
Please let me know what you prefer, and I'll update the PR.

Please make sure the below checklist is followed for Pull Requests.

  • Travis tests are green
  • Tests are added where necessary
  • Documentation is added/updated where necessary
  • Coding Rules & Commit Guidelines as per our CONTRIBUTING.md document are followed

@MathieuAA
Copy link
Member

Thanks @murdos I'll test this out ASAP!

Copy link
Member

@MathieuAA MathieuAA left a comment

Choose a reason for hiding this comment

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

Except the missing test, everything's perfect

@@ -316,6 +316,9 @@ function addEntityOptionsToJDL(entity, entityName) {
if (entity.jpaMetamodelFiltering === true) {
addUnaryOptionToJDL(FILTER, entityName);
}
if (entity.readOnly === true) {
Copy link
Member

Choose a reason for hiding this comment

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

missing a test here:

  • add in the Employee.json file (used in this converter's test file) this entry "readOnly": true.
  • in the test, l.156, add this:
          expect(
            jdlObject
              .getOptions()
              .filter(option => option.name === UnaryOptions.READ_ONLY && option.entityNames.has('Employee')).length
          ).to.equal(1);

@murdos
Copy link
Contributor Author

murdos commented Sep 22, 2019

Thanks. I'm going to add the missing test.
So do we keep readOnly or readonly?

@MathieuAA
Copy link
Member

readOnly is good, let's keep it this way

@murdos
Copy link
Contributor Author

murdos commented Sep 22, 2019

Ok. Missing test has been added

@MathieuAA
Copy link
Member

Thanks :)

@MathieuAA
Copy link
Member

Alright, let's merge it. Thanks @murdos!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add JDL keyword "readonly"
2 participants