Skip to content

Add data access request support#27879

Open
anuj-kumary wants to merge 19 commits intomainfrom
harshach/data-access-tasks
Open

Add data access request support#27879
anuj-kumary wants to merge 19 commits intomainfrom
harshach/data-access-tasks

Conversation

@anuj-kumary
Copy link
Copy Markdown
Member

@anuj-kumary anuj-kumary commented May 4, 2026

Describe your changes:

Fixes

I worked on ... because ...

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

Summary by Gitar

  • UI and SDK enhancements:
    • Updated DataAssetsHeader to use isDarApprovalActive for managing request state validation.
    • Implemented isDarApprovalActive utility in TasksUtils to handle duration and expiration logic for DAR approvals.

This will update automatically on new commits.

@anuj-kumary anuj-kumary requested a review from a team as a code owner May 4, 2026 05:40
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@anuj-kumary anuj-kumary added the safe to test Add this label to run secure Github workflows on PRs label May 4, 2026
Comment on lines +210 to +224
Map<String, Object> payload = new HashMap<>();
payload.put("accessType", accessType.value());
payload.put("requestedAccess", permission.value());
payload.put("reason", reason);
if (duration != null) {
payload.put("duration", duration.toString());
}
if (columns != null && !columns.isEmpty()) {
payload.put("columns", columns);
}

CreateTask request =
new CreateTask()
.withCategory(TaskCategory.DataAccess)
.withType(TaskEntityType.DataAccessRequest)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Quality: Tasks.java uses mutable HashMap for payload — prefer unmodifiable

Per project standards ('Return unmodifiable collections'), the requestDataAccess method builds a mutable HashMap and passes it directly to the builder. After construction, the map should be made unmodifiable to prevent accidental mutation downstream.

Suggested fix:

Wrap the payload before passing it:
`.withPayload(Collections.unmodifiableMap(payload));`
Or build with `Map.of(...)` / `Map.ofEntries(...)` using conditional logic for optional keys.

Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

✅ TypeScript Types Auto-Updated

The generated TypeScript types have been automatically updated based on JSON schema changes in this PR.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 62%
62.45% (62922/100755) 42.77% (33911/79285) 45.77% (10035/21924)

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 4, 2026

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

🔴 Playwright Results — 3 failure(s), 10 flaky

✅ 3209 passed · ❌ 3 failed · 🟡 10 flaky · ⏭️ 69 skipped

Shard Passed Failed Flaky Skipped
🟡 Shard 1 298 0 1 4
🔴 Shard 2 747 1 5 9
🟡 Shard 3 745 0 1 7
✅ Shard 5 687 0 0 41
🔴 Shard 6 732 2 3 8

Genuine Failures (failed on all attempts)

Features/IncidentManager.spec.ts › Next, Previous and page indicator (shard 2)

Pages/Tasks.spec.ts › Resolve task with rejection (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBe�[2m(�[22m�[32mexpected�[39m�[2m) // Object.is equality�[22m

Expected: �[32m"Open"�[39m
Received: �[31mundefined�[39m
Pages/Tasks.spec.ts › All built-in task categories can be created (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBe�[2m(�[22m�[32mexpected�[39m�[2m) // Object.is equality�[22m

Expected: �[32m"DataAccess"�[39m
Received: �[31mundefined�[39m
🟡 10 flaky test(s) (passed on retry)
  • Pages/UserCreationWithPersona.spec.ts › Create user with persona and verify on profile (shard 1, 1 retry)
  • Features/ActivityAPI.spec.ts › Activity event is created when description is updated (shard 2, 1 retry)
  • Features/ActivityAPI.spec.ts › Activity event is created when tags are added (shard 2, 1 retry)
  • Features/ColumnBulkOperations.spec.ts › should update display name and propagate to all occurrences (shard 2, 1 retry)
  • Features/IncidentManager.spec.ts › Complete Incident lifecycle with table owner (shard 2, 2 retries)
  • Features/IncidentManager.spec.ts › Verify filters in Incident Manager's page (shard 2, 2 retries)
  • Features/RTL.spec.ts › Verify Following widget functionality (shard 3, 1 retry)
  • Pages/Lineage/LineageFilters.spec.ts › Verify lineage schema filter selection (shard 6, 1 retry)
  • Pages/Lineage/LineageRightPanel.spec.ts › Verify custom properties tab IS visible for supported type: searchIndex (shard 6, 1 retry)
  • Pages/TasksUIFlow.spec.ts › Create and resolve description task for Topic via UI (shard 6, 1 retry)

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented May 4, 2026

Code Review 👍 Approved with suggestions 3 resolved / 4 findings

Implements Data Access Request task support, updating the corresponding schemas and TypeScript types while resolving build-breaking widget import issues and unused variables. Consider updating Tasks.java to use an unmodifiable map for the request payload to align with project standards.

💡 Quality: Tasks.java uses mutable HashMap for payload — prefer unmodifiable

📄 openmetadata-sdk/src/main/java/org/openmetadata/sdk/fluent/Tasks.java:210-224

Per project standards ('Return unmodifiable collections'), the requestDataAccess method builds a mutable HashMap and passes it directly to the builder. After construction, the map should be made unmodifiable to prevent accidental mutation downstream.

Suggested fix
Wrap the payload before passing it:
`.withPayload(Collections.unmodifiableMap(payload));`
Or build with `Map.of(...)` / `Map.ofEntries(...)` using conditional logic for optional keys.
✅ 3 resolved
Bug: Widget imports deleted files — build will break

📄 openmetadata-ui/src/main/resources/ui/src/components/DataMarketplace/MarketplaceDataAccessRequestsWidget/MarketplaceDataAccessRequestsWidget.component.tsx:31 📄 openmetadata-ui/src/main/resources/ui/src/components/DataMarketplace/MarketplaceDataAccessRequestsWidget/MarketplaceDataAccessRequestsWidget.component.tsx:35 📄 openmetadata-ui/src/main/resources/ui/src/components/DataMarketplace/MarketplaceDataAccessRequestsWidget/MarketplaceDataAccessRequestsWidget.component.tsx:104
Commit f36bdf4 removed DataAccessRequestUtils.ts and DataAccessRequestDetailDrawer.component.tsx, but MarketplaceDataAccessRequestsWidget.component.tsx (added in the second commit) still imports both:

  • Line 31: getDisplayStatus from '../../../utils/DataAccessRequest/DataAccessRequestUtils'
  • Line 35: DataAccessRequestDetailDrawer from '../../DataAccessRequest/DataAccessRequestDetailDrawer/DataAccessRequestDetailDrawer.component'

Additionally, line 104 references ROUTES.DATA_ACCESS_REQUESTS which does not exist in constants.ts.

This will cause a TypeScript compilation failure — the frontend build will not succeed.

Quality: Unused variables in EntityRightPanel after destructuring

📄 openmetadata-ui/src/main/resources/ui/src/components/Entity/EntityRightPanel/EntityRightPanel.tsx:50 📄 openmetadata-ui/src/main/resources/ui/src/components/Entity/EntityRightPanel/EntityRightPanel.tsx:67 📄 openmetadata-ui/src/main/resources/ui/src/components/Entity/EntityRightPanel/EntityRightPanel.tsx:85-99
New destructured variables (name, displayName, columns, reviewers) and computed values (availableColumns, reviewerFqns) are added to EntityRightPanel.tsx but never referenced in the component's JSX or logic. This will likely trigger lint warnings (no-unused-vars) and adds dead code. The new editAllPermission prop is similarly unused.

Quality: Hardcoded boxShadow value instead of design token

📄 openmetadata-ui/src/main/resources/ui/src/components/DataMarketplace/MarketplaceDataAccessRequestsWidget/MarketplaceDataAccessRequestsWidget.component.tsx:133-135
Line 134 uses style={{ boxShadow: '0 0 10px 0 rgba(21, 17, 68, 0.06)' }} which violates the project standard of using semantic CSS custom properties. While no exact --shadow-* CSS custom property exists today, the project has LESS shadow variables and the standard says to avoid hardcoded values.

🤖 Prompt for agents
Code Review: Implements Data Access Request task support, updating the corresponding schemas and TypeScript types while resolving build-breaking widget import issues and unused variables. Consider updating Tasks.java to use an unmodifiable map for the request payload to align with project standards.

1. 💡 Quality: Tasks.java uses mutable HashMap for payload — prefer unmodifiable
   Files: openmetadata-sdk/src/main/java/org/openmetadata/sdk/fluent/Tasks.java:210-224

   Per project standards ('Return unmodifiable collections'), the `requestDataAccess` method builds a mutable `HashMap` and passes it directly to the builder. After construction, the map should be made unmodifiable to prevent accidental mutation downstream.

   Suggested fix:
   Wrap the payload before passing it:
   `.withPayload(Collections.unmodifiableMap(payload));`
   Or build with `Map.of(...)` / `Map.ofEntries(...)` using conditional logic for optional keys.

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 4, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants