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

Follow contact permissions when importing contacts #14426

Open
wants to merge 10 commits into
base: 5.2
Choose a base branch
from

Conversation

fedys
Copy link
Member

@fedys fedys commented Jan 2, 2025

Q A
Bug fix? (use the a.b branch) 🟢
New feature/enhancement? (use the a.x branch) 🔴
Deprecations? 🔴
BC breaks? (use the c.x branch) 🔴
Automated tests included? 🟢
Related user documentation PR URL
Related developer documentation PR URL
Issue(s) addressed

Description

This PR adds permission checks during contact and company imports.


📋 Steps to test this PR:

  1. Create a role that does not have permissions for editing contacts but has permissions to create contacts.
  2. Login as a user that has the above role assigned to them.
  3. Create a new contact import that contains rows for both inserts and updates.
  4. Wait till the import is imported.
  5. Any CSV rows containing updates should be ignored.
  6. There should be error messages like User '<username>' has insufficient permissions for all the rows containing updates.
  7. Validate that permissions are checked against a user who last updated a given import.
  8. Validate the above steps for company imports at /s/companies/import/new. The company import follows the same Contact Permissions. There are no separate Company Permissions.

@fedys fedys requested a review from escopecz January 2, 2025 14:19
@fedys
Copy link
Member Author

fedys commented Jan 2, 2025

@escopecz The failing tests stop failing with RuntimeException: Import does not have "modifiedBy" property set. once https://github.com/acquia/mc-cs/pull/1534 is pushed.

@escopecz
Copy link
Member

escopecz commented Jan 6, 2025

This will be ready for testing once #14433 is merged and this PR rebased.

@escopecz escopecz added the blocked Something blocks this PR/issue (e.g. waiting for another PR to be merged) label Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Something blocks this PR/issue (e.g. waiting for another PR to be merged)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants