Switch branch Action #286
Switch branch Action #286
Conversation
|
Remove the backlog link from the description. Instead add some description of what the pull request solves.
|
org.abapgit.adt.ui/src/org/abapgit/adt/ui/internal/repositories/AbapGitView.java
Outdated
Show resolved
Hide resolved
org.abapgit.adt.ui/src/org/abapgit/adt/ui/internal/repositories/actions/SwitchbranchAction.java
Outdated
Show resolved
Hide resolved
...bapgit/adt/ui/internal/repositories/wizards/AbapGitWizardPageBranchSelectionCredentials.java
Outdated
Show resolved
Hide resolved
...dt.ui/src/org/abapgit/adt/ui/internal/repositories/wizards/AbapGitWizardBranchSelection.java
Outdated
Show resolved
Hide resolved
...i/src/org/abapgit/adt/ui/internal/repositories/wizards/AbapGitWizardPageBranchSelection.java
Outdated
Show resolved
Hide resolved
...dt.ui/src/org/abapgit/adt/ui/internal/repositories/wizards/AbapGitWizardBranchSelection.java
Outdated
Show resolved
Hide resolved
...dt.ui/src/org/abapgit/adt/ui/internal/repositories/wizards/AbapGitWizardBranchSelection.java
Outdated
Show resolved
Hide resolved
...dt.ui/src/org/abapgit/adt/ui/internal/repositories/wizards/AbapGitWizardBranchSelection.java
Outdated
Show resolved
Hide resolved
...dt.ui/src/org/abapgit/adt/ui/internal/repositories/wizards/AbapGitWizardBranchSelection.java
Outdated
Show resolved
Hide resolved
...dt.ui/src/org/abapgit/adt/ui/internal/repositories/wizards/AbapGitWizardBranchSelection.java
Outdated
Show resolved
Hide resolved
|
@aarnapant-sap Please also test the solution in a ABAP Cloud system, involving transport requests. |
org.abapgit.adt.ui/src/org/abapgit/adt/ui/internal/repositories/actions/SwitchbranchAction.java
Outdated
Show resolved
Hide resolved
org.abapgit.adt.ui/src/org/abapgit/adt/ui/internal/wizards/AbapGitWizardBranchSelection.java
Outdated
Show resolved
Hide resolved
org.abapgit.adt.ui/src/org/abapgit/adt/ui/internal/wizards/AbapGitWizardBranchSelection.java
Outdated
Show resolved
Hide resolved
org.abapgit.adt.ui/src/org/abapgit/adt/ui/internal/wizards/AbapGitWizardBranchSelection.java
Outdated
Show resolved
Hide resolved
org.abapgit.adt.ui/src/org/abapgit/adt/ui/internal/wizards/AbapGitWizardBranchSelection.java
Outdated
Show resolved
Hide resolved
...dt.ui/src/org/abapgit/adt/ui/internal/wizards/AbapGitWizardPageRepositoryAndCredentials.java
Outdated
Show resolved
Hide resolved
org.abapgit.adt.ui/src/org/abapgit/adt/ui/internal/wizards/AbapGitWizardBranchSelection.java
Outdated
Show resolved
Hide resolved
....ui/src/org/abapgit/adt/ui/internal/wizards/AbapGitWizardPageBranchSelectionCredentials.java
Outdated
Show resolved
Hide resolved
| public class SwitchbranchAction extends Action { | ||
|
|
||
| private IRepository selRepo; | ||
| private final IViewPart AbapGitView; |
There was a problem hiding this comment.
Can keep this view variable typed to AbapGitView itself i.e. private final AbapGitView view.
| public AbapGitWizardPageSwitchBranchAndPackage(IProject project, String destination, CloneData cloneData, Boolean pullAction) { | ||
| super(project, destination, cloneData, pullAction); |
There was a problem hiding this comment.
Like in case of credentials page, here as well the pullAction will be false always.
We can remove the parameter pullAction in the constructor and set pullAction as false for the super constructor.
| if (!getPackageAndRepoType()) { | ||
| this.isPackageValid = false; | ||
| this.packageErrorMessage = NLS.bind(Messages.AbapGitWizardSwitch_branch_package_ref_not_found_error, | ||
| this.selRepoData.getPackage()); | ||
| } |
There was a problem hiding this comment.
The error message will be set even in case there was an exception from backend while fetching the repo type as the logic depends only on whether the method getPackageAndRepoType returns false.
| ((WizardPage) getContainer().getCurrentPage()).setPageComplete(false); | ||
| ((WizardPage) getContainer().getCurrentPage()).setMessage(e.getMessage(), DialogPage.ERROR); |
There was a problem hiding this comment.
This will lead to runtime error as there is no Wizard yet initialized, similar to the package error handling.
So in this case as well, better to show the error in MessageDialog.
There was a problem hiding this comment.
Instead of handling the exception here, let's propagate the exception to caller & handle the exception where we open the wizard i.e. in SwitchBranchAction.
So even before opening the wizard, if there is an exception while creating the object of AbapGitWizardSwitchBranch we show error in MessageDialog.
| if (packageServiceUI.packageExists(AbapGitWizardSwitchBranch.this.destination, packageName, monitor)) { | ||
| List<IAdtObjectReference> packageRefs = packageServiceUI.find(AbapGitWizardSwitchBranch.this.destination, packageName, monitor); | ||
| AbapGitWizardSwitchBranch.this.cloneData.packageRef = packageRefs.stream().findFirst().orElse(null); | ||
| } |
There was a problem hiding this comment.
In case the package does not exist, let's create new exception class and throw this new exception. Check more comment below.
| if (!this.isPackageValid) { | ||
| // show error message | ||
| Shell shell = PlatformUI.getWorkbench().getActiveWorkbenchWindow().getShell(); | ||
| MessageDialog.openError(shell, "Error", this.packageErrorMessage != null ? this.packageErrorMessage : "Unknown error"); //$NON-NLS-1$ //$NON-NLS-2$ | ||
|
|
||
| if (getContainer() instanceof WizardDialog) { | ||
| ((WizardDialog) getContainer()).close(); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
Instead of handling the package validation in addPages, it would make more sense if we performed the validation before even opening the wizard.
Hence, the new exception mentioned above.
In case the package ref is initial, instead of handling it in addPages, an exception would be thrown that can be handled in SwitchBranchAction & this logic can be moved to SwitchBranchAction to handle the new exception.
This is a cleaner approach.
| public class TestPdeAbapGitRepositoriesSelectionWizard { | ||
|
|
There was a problem hiding this comment.
- More test cases can be covered like invalid package or exception while fetching repo type.
- Also a UI test can be added to open the wizard, validate the fields in first page, then navigate to next page and validate if the branches are correctly set as input for combo viewer, and switch the branch in combo viewer. The Finish action can be skipped.
Switch Branch Functionality in AbapGit