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

Align permission API's with Java/Swift #2036

Merged
merged 14 commits into from
Oct 4, 2018
Merged

Conversation

cmelchior
Copy link
Contributor

@cmelchior cmelchior commented Sep 20, 2018

This PR aligns the JS API with Java/Swift by introducing a number of helper methods.

It adds

Realm.permissions()
Realm.permissions(type)
Realm.Permissions.Realm.findOrCreate(roleName)
Realm.Permissions.Class.findOrCreate(roleName)

TODO:

  • Fix changelog
  • Add permissions() method + test
  • Add findOrCreate(roleName) + tests

In order for Realm to pick up extra methods on model classes we have to register the Javascript constructor function with Realm. This, unfortunately, means I have to revert the change by Nabil and move it back to JS land, but it is done in a way compatible with the C++ implementation so that new Realm(config) will also include the schema. This is done by adding the internal _constructor method as wrapping the original constructor itself seemed to cause other places in our code base (and possible client code as well).

@cmelchior
Copy link
Contributor Author

Ready for review

Copy link
Contributor

@kneth kneth left a comment

Choose a reason for hiding this comment

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

Mostly changelog and documentation comments.

👍

* If the Permission object is created because one didn't exist already, it will be
* created with all privileges disabled.
*
* If the Role object is created because one didn't exists, it will be created
Copy link
Contributor

Choose a reason for hiding this comment

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

"exists" -> "exist"


/**
* Finds the Class-level permissions associated with the named Role. If either the role or the permission
* object doesn't exists, it will be created.
Copy link
Contributor

Choose a reason for hiding this comment

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

"exists" -> "exist"


/**
* Finds the Realm-level permissions associated with the named Role. If either the role or the permission
* object doesn't exists, it will be created.
Copy link
Contributor

Choose a reason for hiding this comment

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

"exists" -> "exist"

CHANGELOG.md Outdated
@@ -1,8 +1,13 @@
x.x.x Release notes (yyyy-MM-dd)
=============================================================
## Enhancements
* None

* Added support for finding Realm-level permissions in Query-based Realms using `realm.getPermissions()` ([#2036]
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move . to end of sentence: "Added ... using realm.getPermissions(). ([..."

docs/realm.js Outdated
/**
* Returns the fine-grained permissions object associated with either the Realm itself or a Realm model class.
*
* @param {Realm~ObjectType} [arg] - If no argument is provided, the Realm-level permissions are are returned.
Copy link
Contributor

Choose a reason for hiding this comment

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

"are are" -> "are"

}

// Realm instance methods that are available always available
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove one "available"

@bmunkholm
Copy link
Contributor

Changelog is fine.

@cmelchior cmelchior merged commit 434e8ca into master Oct 4, 2018
@cmelchior cmelchior deleted the cm/add-get-permission-apis branch October 4, 2018 08:09
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants