ENG-2645: FE Support Jira Ticket integration oauth and test flows#7610
ENG-2645: FE Support Jira Ticket integration oauth and test flows#7610eastandwestwind merged 18 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
✏️ Tip: You can disable in-progress messages and the fortune message in your review settings. Tip CodeRabbit can enforce grammar and style rules using `languagetool`.Configure the 📝 WalkthroughWalkthroughAdds Jira ticket integration with OAuth: new ConnectionType, UI components and hooks for Jira authorization, backend OAuth initiation endpoint, feature flag gating, and UI/authorization flow adjustments across admin UI and backend. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as Admin UI
participant API as API Server
participant Jira as Jira OAuth
participant DB as Database
User->>UI: Click "Authorize with Jira"
UI->>API: POST /plus/oauth/jira/initiate {connection_key}
API->>DB: Read connection & secrets
DB-->>API: connection data
API-->>UI: {authorization_url}
UI->>Jira: Redirect user to authorization_url
Jira->>User: User authenticates & approves
Jira-->>UI: Redirect back with jira_auth param
UI->>API: Refresh connection/test data
API->>DB: Check secrets for access_token
DB-->>API: access_token present
API-->>UI: Connection authorized
UI->>User: Show authorization result (authorized / not authorized)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR wires up the Jira OAuth connection UI in the admin interface — including OAuth initiation, callback handling, connection status display, and feature-flag gating — and extends the backend
Confidence Score: 2/5
Important Files Changed
|
| """Returns True if the connection config has an access token, used for OAuth2 connections""" | ||
|
|
||
| if self.connection_type == ConnectionType.jira_ticket: | ||
| return bool(self.secrets and "access_token" in self.secrets) |
There was a problem hiding this comment.
Access token presence check allows falsy values
The expression "access_token" in self.secrets only verifies that the key exists in the dict, not that the value is truthy. A stored access_token of None, "", or 0 would cause authorized to return True even though there is no usable token.
| return bool(self.secrets and "access_token" in self.secrets) | |
| return bool(self.secrets and self.secrets.get("access_token")) |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
clients/admin-ui/src/pages/integrations/index.tsx (1)
201-230:⚠️ Potential issue | 🟡 MinorCheck authorization before the last-test tooltip.
After a Jira connection is disconnected,
last_test_timestampcan still be populated from the earlier healthy run. With the current ordering, the tag showsUnauthorizedbut the tooltip still saysLast connection: ....💡 Suggested fix
- const tooltipText = record.last_test_timestamp - ? `Last connection: ${formatDate(record.last_test_timestamp)}` - : isJiraTicket && !record.authorized - ? "Jira connection has not been authorized" - : "The connection has not been tested"; + const tooltipText = + isJiraTicket && !record.authorized + ? "Jira connection has not been authorized" + : record.last_test_timestamp + ? `Last connection: ${formatDate(record.last_test_timestamp)}` + : "The connection has not been tested";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/admin-ui/src/pages/integrations/index.tsx` around lines 201 - 230, The tooltip logic currently prefers record.last_test_timestamp over authorization, causing an Unauthorized Jira connection to still show a "Last connection" tooltip; update the tooltipText computation (near variables isJiraTicket, getConnectionStatus, tooltipText, and record) to check isJiraTicket && !record.authorized first and return "Jira connection has not been authorized" in that case, otherwise fall back to the last_test_timestamp message or the default "The connection has not been tested".
🧹 Nitpick comments (1)
clients/admin-ui/src/pages/integrations/[id].tsx (1)
132-138: Side effect during render mirrors existing pattern but could be improved.Calling
router.pushduring render is technically a side effect. This follows the existing pattern at lines 125-130, but both could benefit from being moved into auseEffectto avoid potential React warnings or unexpected behavior during concurrent rendering.Since this matches the existing code style in the file, this is acceptable for consistency, but consider refactoring both guards to use
useEffectin a follow-up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/admin-ui/src/pages/integrations/`[id].tsx around lines 132 - 138, Render currently performs a side-effect by calling router.push when connection exists, connection.connection_type === ConnectionType.JIRA_TICKET, and !alphaJiraIntegration; move this navigation logic into a React useEffect to prevent side-effects during render: create a useEffect that depends on connection, connection.connection_type, alphaJiraIntegration and router (or router.asPath) and inside it check the same condition and call router.push(INTEGRATION_MANAGEMENT_ROUTE) so the redirect runs as an effect rather than during render.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@clients/admin-ui/src/features/datastore-connections/system_portal_config/forms/fields/DatasetConfigField/useDatasetConfigField.tsx`:
- Around line 25-28: The component uses the query result variable data from
useGetConnectionConfigDatasetConfigsQuery(connectionKey, { skip: !connectionKey
}) which can retain the previous successful result when skipped; change to read
currentData instead (or clear derived dataset options when !connectionKey) so
that when connectionKey is falsy the datasets become undefined rather than stale
values—update the useGetConnectionConfigDatasetConfigsQuery usage to extract
currentData and use that in the downstream logic that computes dataset options
(or add an explicit guard that sets options to []/undefined when
!connectionKey).
In `@clients/admin-ui/src/features/integrations/hooks/useJiraAuthorization.ts`:
- Around line 24-43: The handleAuthorize function is not re-entrant safe; add a
guard to prevent concurrent calls (use the existing isLoading from
useInitiateJiraOAuth or a local ref flag like isAuthorizingRef) so if isLoading
or isAuthorizingRef.current is true you return early, set the flag true before
calling initiateJiraOAuth({ connection_key: connection.key }) and clear it in a
finally block, and only set window.location.href after a successful single
response; update handleAuthorize to check and set this guard around the call to
initiateJiraOAuth/unwrap to avoid minting multiple OAuth states and following
stale redirects.
---
Outside diff comments:
In `@clients/admin-ui/src/pages/integrations/index.tsx`:
- Around line 201-230: The tooltip logic currently prefers
record.last_test_timestamp over authorization, causing an Unauthorized Jira
connection to still show a "Last connection" tooltip; update the tooltipText
computation (near variables isJiraTicket, getConnectionStatus, tooltipText, and
record) to check isJiraTicket && !record.authorized first and return "Jira
connection has not been authorized" in that case, otherwise fall back to the
last_test_timestamp message or the default "The connection has not been tested".
---
Nitpick comments:
In `@clients/admin-ui/src/pages/integrations/`[id].tsx:
- Around line 132-138: Render currently performs a side-effect by calling
router.push when connection exists, connection.connection_type ===
ConnectionType.JIRA_TICKET, and !alphaJiraIntegration; move this navigation
logic into a React useEffect to prevent side-effects during render: create a
useEffect that depends on connection, connection.connection_type,
alphaJiraIntegration and router (or router.asPath) and inside it check the same
condition and call router.push(INTEGRATION_MANAGEMENT_ROUTE) so the redirect
runs as an effect rather than during render.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 669b2818-366a-4b94-a893-010d5cfacb17
📒 Files selected for processing (17)
changelog/7610.yamlclients/admin-ui/src/features/datastore-connections/system_portal_config/forms/fields/DatasetConfigField/useDatasetConfigField.tsxclients/admin-ui/src/features/integrations/ConnectionStatusNotice.tsxclients/admin-ui/src/features/integrations/IntegrationBox.tsxclients/admin-ui/src/features/integrations/add-integration/ConfigureIntegrationForm.tsxclients/admin-ui/src/features/integrations/add-integration/SelectIntegrationType.tsxclients/admin-ui/src/features/integrations/add-integration/allIntegrationTypes.tsxclients/admin-ui/src/features/integrations/hooks/useFeatureBasedTabs.tsxclients/admin-ui/src/features/integrations/hooks/useJiraAuthorization.tsclients/admin-ui/src/features/integrations/integration-type-info/jiraTicketInfo.tsxclients/admin-ui/src/features/integrations/setup-steps/hooks/useAuthorizeIntegrationStep.tsxclients/admin-ui/src/features/plus/plus.slice.tsclients/admin-ui/src/flags.jsonclients/admin-ui/src/pages/integrations/[id].tsxclients/admin-ui/src/pages/integrations/index.tsxclients/admin-ui/src/types/api/models/ConnectionType.tssrc/fides/api/models/connectionconfig.py
...e-connections/system_portal_config/forms/fields/DatasetConfigField/useDatasetConfigField.tsx
Outdated
Show resolved
Hide resolved
clients/admin-ui/src/features/integrations/hooks/useJiraAuthorization.ts
Show resolved
Hide resolved
ea88cb8 to
c92df4b
Compare
JadeCara
left a comment
There was a problem hiding this comment.
Tried it locally! No issues :)
jpople
left a comment
There was a problem hiding this comment.
Code looks good and did some manual testing, everything working as expected for me.
Ticket [https://ethyca.atlassian.net/browse/ENG-2645] (fides portion)
Please test alongside https://github.com/ethyca/fidesplus/pull/3205
Description Of Changes
Adds the Jira OAuth Connection UI to the admin interface, gated behind the
alphaJiraIntegrationfeature flag. Admins can connect to Jira via OAuth, view connection status, test connectivity, and disconnect. The backendauthorizedproperty onConnectionConfigis extended to supportjira_ticketconnections by checking for anaccess_tokenin secrets.Code Changes
New files:
useJiraAuthorization.ts: Custom hook for initiating Jira OAuth flow viaPOST /plus/oauth/jira/initiateand redirecting to AtlassianjiraTicketInfo.tsx: Integration type metadata for the Jira Ticket integration (name, category, tags, overview text)Frontend integration wiring:
ConnectionType.ts: AddedJIRA_TICKETenum valueallIntegrationTypes.tsx: RegisteredJIRA_TICKETinINTEGRATION_TYPE_MAPplus.slice.ts: AddedinitiateJiraOAuthmutation endpointSelectIntegrationType.tsx: Filtered Jira from add-integration modal when flag is offindex.tsx(integrations list): Filtered Jira from connection queries when flag is off; shows "Unauthorized" status for unauthorized Jira connections[id].tsx(integration detail): Added OAuth callback handling (?jira_auth=success/error), Jira auth hook wiring, feature flag gating, removed "Linked system" tab for JiraConnection status and authorization UI:
ConnectionStatusNotice.tsx: Shows "Connection not authorized" for unauthorized Jira connections instead of "Connection not tested"IntegrationBox.tsx: Dispatches touseJiraAuthorizationfor Jira connections; shows "Authorize with Jira" buttonuseFeatureBasedTabs.tsx: PassesconnectionTypeto status notice; shows Jira-specific button labeluseAuthorizeIntegrationStep.tsx: Shows "Authorize integration" setup step for Jira connectionsConfiguration form fixes:
ConfigureIntegrationForm.tsx: ExcludedJIRA_TICKETfrom secrets form and system linking dropdown (uses OAuth, not direct secrets)useDatasetConfigField.tsx: Skip dataset config query when connection key is emptyBackend:
connectionconfig.py: Extendedauthorizedproperty to returnTrueforjira_ticketconnections with anaccess_tokenin secretsSteps to Confirm
Prerequisites:
client_idandclient_secretfrom Atlassian Developer Console > Settingshttp://localhost:8080/api/v1/plus/oauth/jira/callbackPre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and worksSummary by CodeRabbit
New Features
Documentation