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.

Sign up for GitHub

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

Jump to bottom

[BUG] Appwrite dataProvider meta.permissions (@refinedev/appwrite) #5767

Closed
abdelrahman-essawy opened this issue Mar 18, 2024 · 7 comments · Fixed by #5886
Closed

[BUG] Appwrite dataProvider meta.permissions (@refinedev/appwrite) #5767

abdelrahman-essawy opened this issue Mar 18, 2024 · 7 comments · Fixed by #5886
Assignees
@abdelrahman-essawy
Labels
bug Something isn't working
Milestone
May Release

Comments

@abdelrahman-essawy
Copy link
Contributor

abdelrahman-essawy commented Mar 18, 2024 •
edited

Describe the bug

Hello,
how to override these default permissions in appwrite dataProvider? I don't want public permissions, only the permissions i set in meta at creation/edition.

  const { saveButtonProps, formProps, form } = useForm<HttpError>({
    meta: {
      readPermissions: [Permission.read(Role.user(identity.$id))],
      writePermissions: [Permission.write(Role.user(identity.$id))],
    },
  });

I was able to walk-around it by making custom dataProviderWrapper that overrides the original create method

  return {
  ...originalDataProvider,
  create: async ({ resource, variables, meta }) => {
    const permissions = [
      // Permission.read(Role.any()),
      // Permission.write(Role.any()),
      ...(meta?.readPermissions ?? [Permission.read(Role.any())]),
      ...(meta?.writePermissions ?? [Permission.write(Role.any())]),
    ];

Steps To Reproduce

  1. create new app
npm create refine-app@latest -- --example data-provider-appwrite
  1. use the following snippet with any roles.
  const { saveButtonProps, formProps, form } = useForm<HttpError>({
    meta: {
      readPermissions: [Permission.read(Role.user(identity.$id))],
      writePermissions: [Permission.read(Role.user(identity.$id))],
    },
  });

Expected behavior

expected behavior is to be able to complete override the meta params.

Packages

    "@refinedev/antd": "^5.37.5",
    "@refinedev/appwrite": "^6.4.7",
    "@refinedev/core": "^4.48.0",
    "@refinedev/devtools": "^1.1.35",
    "@refinedev/kbar": "^1.3.7",
    "@refinedev/nextjs-router": "^6.0.1",
    "@refinedev/react-hook-form": "^4.8.15",

Additional Context

sample after override

export const customDataProvider = (
  appwriteClient: Appwrite,
  options: { databaseId: string } = { databaseId: 'default' }
): Required<DataProvider> => {
  const { databaseId } = options;
  const database = new Databases(appwriteClient);
  const originalDataProvider: Required<DataProvider> = dataProvider(
    appwriteClient,
    {
      databaseId,
    }
  );

  return {
    ...originalDataProvider,
    create: async ({ resource, variables, meta }) => {
      const permissions = [
        // Permission.read(Role.any()),
        // Permission.write(Role.any()),
        ...(meta?.readPermissions ?? [Permission.read(Role.any())]), // new permisions
        ...(meta?.writePermissions ?? [Permission.write(Role.any())]), // new permisions
      ];

      const { $id, ...restData } = await database.createDocument(
        databaseId,
        resource,
        meta?.documentId ?? ID.unique(),
        variables as unknown as object,
        permissions
      );

      return {
        data: {
          id: $id,
          ...restData,
        },
      } as any;
    },
}
@abdelrahman-essawy abdelrahman-essawy added the bug Something isn't working label Mar 18, 2024
@BatuhanW
Copy link
Member

Hey @abdelrahman-essawy, it seems appwrite data provider appends permissions, on top of the default ones. It could be updated to have a logic, if user provides permissions, it only uses them.

As a workaround, you can swizzle the data provider and use it. See the documentation here.

@abdelrahman-essawy
Copy link
Contributor Author

@BatuhanW Great!, I will be more than happy to open PR for that, if that's okay.

@BatuhanW
Copy link
Member

@abdelrahman-essawy sure thing. Only concern is, this could be a breaking change for existing users (Not sure if it's a valid case tho).

Perhaps, we could add defaultPermissions to the 2nd argument (options), with default value having Permission.read(Role.any()). This way, existing users wouldn't be affected, and also if someone is using same permissions everywhere, they wouldn't need to pass it to every hook.

Assigning issue to you, looking forward to your PR!

@BatuhanW BatuhanW added this to the April Release milestone Mar 19, 2024
@BatuhanW
Copy link
Member

BatuhanW commented Mar 19, 2024 •
edited

Hey @abdelrahman-essawy discussed with the team, and decided to go with this implementation.

  • Let's remove all hard-coded inline Permission.read(Role.any()), Permission.write(Role.any()) from all methods.
  • Let's have options.defaultReadPermissions, options.defaultWritePermissions argument as arrays, but default values should be empty arrays. (Unline what I said in the previous comment.)
  • If user passes these permissions from meta, let's use them, ignoring options.defaultReadPermissions and/or options.defaultReadPermissions completely.

@abdelrahman-essawy
Copy link
Contributor Author

abdelrahman-essawy commented Mar 19, 2024 •
edited

but default values should be empty arrays. (Unline what I said in the previous comment.)

isn't that a breaking change? users who didn't pass anything before, expects to have Permission.read(Role.any()), Permission.write(Role.any()) passed by default.

notice that if you passed an empty array of permissions

image

appwrite will treat it with no permissions at all.

image

@abdelrahman-essawy abdelrahman-essawy mentioned this issue Mar 19, 2024
Closed
5 tasks
@abdelrahman-essawy abdelrahman-essawy changed the title [BUG] Appwrite dataProvider meta.permissions (@refinedev/appwrite) [BUG] Appwrite dataProvider meta.permissions (@refinedev/appwrite) Mar 20, 2024
@abdelrahman-essawy
Copy link
Contributor Author

but default values should be empty arrays. (Unline what I said in the previous comment.)

isn't that a breaking change? users who didn't pass anything before, expects to have Permission.read(Role.any()), Permission.write(Role.any()) passed by default.

notice that if you passed an empty array of permissions

image

appwrite will treat it with no permissions at all.

image

@BatuhanW hello, what do you think of this ?

@BatuhanW
Copy link
Member

Hey @abdelrahman-essawy, there is no different with passing an empty array and Permission.any from AppWrite's side.

@omeraplak omeraplak modified the milestones: April Release, May Release Apr 3, 2024
@abdelrahman-essawy abdelrahman-essawy mentioned this issue Apr 24, 2024
Merged
5 tasks
@aliemir aliemir closed this as completed May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@abdelrahman-essawy abdelrahman-essawy

Labels
bug Something isn't working
Projects
None yet
Milestone
May Release
4 participants
@omeraplak @aliemir @BatuhanW @abdelrahman-essawy