Fix: Disable policy editing controls on API revision pages #1214
Conversation
Iduranga-Uwanpriya
requested review from
Krishanx92,
lasanthaS,
piyumaldk and
tharikaGitHub
as code owners
|
|
@Iduranga-Uwanpriya also will you be able to attach screenshots to depict the behaviour introduced by this PR? |
Hi @tharikaGitHub , When I ran the app for the first time to test it, everything loaded perfectly. However, after some time, I wasn’t able to load the app properly due to the large chunk file size. I really appreciate all the guidance and references you shared I tried all of them, but unfortunately, even by the end of Hacktoberfest, I couldn’t get it working. I’m also really sorry that I couldn’t provide any screenshots. But I did test the “Add Policy” and other buttons they don’t seem to work in revision mode, as they’re all grayed out. Thank you again for all your help and support! |
| const Policies: React.FC = () => { | ||
|
|
||
| const [api, updateAPI] = useAPI(); | ||
| const isRevision = api.isRevision || Boolean(api.revisionId); |
There was a problem hiding this comment.
Why do we need a check like this? || Boolean(api.revisionId); Wouldn't api.isRevision be sufficient?
| PolicyDropzone={PolicyDropzone} | ||
| listOriginatedFromCommonPolicies={listOriginatedFromCommonPolicies} | ||
| isApiRevision={api.isRevision} | ||
| isApiRevision={disabled || api.isRevision} |
There was a problem hiding this comment.
Do we need this disabled check when api.isRevision check is already there?
| <Grid item> | ||
| <Box p={1} mt={3}> | ||
| {api.isRevision || (settings && settings.portalConfigurationOnlyModeEnabled) || isRestricted(['apim:api_create'], api) ? ( | ||
| {disabled || api.isRevision || (settings && settings.portalConfigurationOnlyModeEnabled) || isRestricted(['apim:api_create'], api) ? ( |
There was a problem hiding this comment.
Do we need disabled check when api.isRevision is checked here?
| fetchPolicies={fetchPolicies} | ||
| DraggablePolicyCard={DraggablePolicyCard} | ||
| isReadOnly={isReadOnly} | ||
| isReadOnly={disabled || isReadOnly} |
There was a problem hiding this comment.
Do we need disabled check when isReadOnly is checked?
2 participants
Description
Fixes issue #3781 where policy editing controls remain enabled on API revision pages, allowing users to attempt modifications even though revisions are read-only.
Problem Statement
When viewing an API revision in the Publisher portal's Policies page, users can interact with editing controls that should be disabled:
This creates a confusing user experience as revisions are immutable snapshots and should not allow any modifications.
Solution
Implemented a comprehensive read-only mode for policy pages when viewing revisions:
Revision Detection: Added logic in
Policies.tsxto detect revision views usingapi.isRevision || Boolean(api.revisionId)Props Propagation: Created a
disabledprop that flows through the component hierarchy to ensure consistent behavior across all policy-related componentsUI Control Management: Updated all interactive elements to respect the disabled state while keeping policy information visible
Changes Made
Core Components Modified:
Policies.tsxisRevisionconstant for revision detectiondisabledstatedisabledprop to child components (PolicyPanel,PolicyList,SaveOperationPolicies)PolicyPanel.tsxdisabledprop to interfacePoliciesSectionPoliciesSection.tsxdisabledprop to interfacePoliciesExpansion(API level) andOperationPolicy(operation level)PoliciesExpansion.tsxdisabledprop to interfaceisApiRevisionprop in shared component to include disabled state:isApiRevision={disabled || api.isRevision}OperationPolicy.tsxdisabledprop to interfacePoliciesExpansionPolicyList.tsxdisabledprop to interfacedisabled={disabled || isRestricted([...])}disabledto allTabPanelcomponents (Request, Response, Fault flows)TabPanel.tsxdisabledprop to interfaceisReadOnlyprop:isReadOnly={disabled || isReadOnly}SaveOperationPolicies.tsxdisabledprop to interfacedisabled || api.isRevision || ...Technical Implementation Details
Design Pattern
The solution follows React's unidirectional data flow pattern, where the disabled state is determined at the top level (
Policies.tsx) and propagates down through props.Shared Component Integration
The implementation integrates with existing shared components (
PoliciesExpansionShared,TabPanelShared) by utilizing their existingisApiRevisionandisReadOnlyprops rather than modifying the shared components themselves.Backward Compatibility
All new props are optional (
disabled?: boolean) with default values, ensuring backward compatibility with existing code.Testing Performed
Compilation
Code Review
Manual Testing Checklist
Due to local development environment issues, comprehensive manual testing is pending. However, the implementation follows established patterns already present in the codebase (e.g.,
SaveOperationPoliciesalready implements revision checking).Expected Behavior (for reviewer verification):
On Revision Pages:
On Regular API Pages (Non-Revision):
Testing Instructions for Reviewers
Prerequisites
Steps to Test
git checkout fix/3781-readonly-policy-tab-v2 cd portals/publisher/src/main/webapp npm ci npm startCreate Test API:
Attach Policies:
Create Revision:
Verify Read-Only Behavior:
Verify Normal API Still Works:
Related Issue
Resolves #3781
Screenshots
Note: Screenshots currently unavailable due to local development environment configuration issues. The code implementation has been completed following established patterns in the codebase.
Checklist
Additional Notes
This implementation maintains the principle of "show the data, disable the actions" - ensuring that policy information remains visible and accessible for reference while preventing any modifications to the immutable revision.
The solution is minimal and focused, touching only the necessary components to achieve the desired functionality without introducing unnecessary complexity or architectural changes.