Skip to content

Commit

Permalink
Quick fix UX improvements (#368)
Browse files Browse the repository at this point in the history
* UX improvements regarding the dependency upgrade process (quick fix): show a progress bar (indicator), refresh the file when the upgrade is done.

* Bug fix - in Go, npm, and Yarn projects, upgrading dependencies worked on the project's root directory only.
  • Loading branch information
asafgabai authored Jul 24, 2023
1 parent 6099dd7 commit 0194daa
Show file tree
Hide file tree
Showing 14 changed files with 60 additions and 36 deletions.
7 changes: 3 additions & 4 deletions src/main/java/com/jfrog/ide/idea/ci/CiManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import com.jfrog.ide.idea.log.Logger;
import com.jfrog.ide.idea.log.ProgressIndicatorImpl;
import com.jfrog.ide.idea.ui.CiComponentsTree;
import com.jfrog.ide.idea.ui.ComponentsTree;
import com.jfrog.ide.idea.ui.menus.filtermanager.CiFilterManager;
import com.jfrog.ide.idea.utils.Utils;
import org.apache.commons.lang3.StringUtils;
Expand Down Expand Up @@ -186,9 +185,9 @@ private boolean scanPreconditionsMet() {
}

private void registerOnChangeHandlers() {
appBusConnection.subscribe(ApplicationEvents.ON_CONFIGURATION_DETAILS_CHANGE, () -> asyncRefreshBuilds());
projectBusConnection.subscribe(ApplicationEvents.ON_BUILDS_CONFIGURATION_CHANGE, () -> asyncRefreshBuilds());
projectBusConnection.subscribe(BuildEvents.ON_SELECTED_BUILD, this::loadBuild);
appBusConnection.subscribe(ApplicationEvents.ON_CONFIGURATION_DETAILS_CHANGE, (ApplicationEvents) this::asyncRefreshBuilds);
projectBusConnection.subscribe(ApplicationEvents.ON_BUILDS_CONFIGURATION_CHANGE, (ApplicationEvents) this::asyncRefreshBuilds);
projectBusConnection.subscribe(BuildEvents.ON_SELECTED_BUILD, (BuildEvents) this::loadBuild);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,7 @@ Set<DescriptorFileTreeNode> getFileDescriptors(PsiElement element) {
Set<DescriptorFileTreeNode> fileDescriptors = new HashSet<>();
Enumeration<TreeNode> roots = ((SortableChildrenTreeNode) componentsTree.getModel().getRoot()).children();
for (TreeNode root : Collections.list(roots)) {
if (root instanceof DescriptorFileTreeNode) {
DescriptorFileTreeNode fileNode = (DescriptorFileTreeNode) root;
if (root instanceof DescriptorFileTreeNode fileNode) {
if (fileNode.getFilePath().equals(element.getContainingFile().getVirtualFile().getPath())) {
fileDescriptors.add(fileNode);
}
Expand Down Expand Up @@ -238,7 +237,7 @@ boolean isNodeMatch(DependencyNode node, String componentName) {
return StringUtils.equals(artifactID, componentName) || impactPath.contains(componentName);
}

abstract UpgradeVersion getUpgradeVersion(String componentName, String fixVersion, Collection<String> issues);
abstract UpgradeVersion getUpgradeVersion(String componentName, String fixVersion, Collection<String> issues, String descriptorPath);

void registerProblem(ProblemsHolder problemsHolder, DependencyNode dependency, PsiElement element, String componentName) {
boolean isTransitive = dependency.isIndirect() || !StringUtils.contains(dependency.getTitle(), componentName);
Expand All @@ -256,8 +255,9 @@ void registerProblem(ProblemsHolder problemsHolder, DependencyNode dependency, P
}
});

String descriptorPath = element.getContainingFile().getVirtualFile().getPath();
fixVersionToCves.asMap().forEach((fixedVersion, issues) -> {
UpgradeVersion upgradeVersion = getUpgradeVersion(dependency.getArtifactId(), fixedVersion, issues);
UpgradeVersion upgradeVersion = getUpgradeVersion(dependency.getArtifactId(), fixedVersion, issues, descriptorPath);
quickFixes.add(upgradeVersion);
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ String createComponentName(PsiElement element) {
}

@Override
UpgradeVersion getUpgradeVersion(String componentName, String fixVersion, Collection<String> issue) {
return new GoUpgradeVersion(componentName, fixVersion, issue);
UpgradeVersion getUpgradeVersion(String componentName, String fixVersion, Collection<String> issue, String descriptorPath) {
return new GoUpgradeVersion(componentName, fixVersion, issue, descriptorPath);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ private String extractExpression(PsiElement argumentList, String name) {
}

@Override
UpgradeVersion getUpgradeVersion(String componentName, String fixVersion, Collection<String> issue) {
UpgradeVersion getUpgradeVersion(String componentName, String fixVersion, Collection<String> issue, String descriptorPath) {
return new GradleGroovyUpgradeVersion(componentName, fixVersion, issue);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ private String extractArgument(KtValueArgument ktValueArgument) {
}

@Override
UpgradeVersion getUpgradeVersion(String componentName, String fixVersion, Collection<String> issue) {
UpgradeVersion getUpgradeVersion(String componentName, String fixVersion, Collection<String> issue, String descriptorPath) {
return new GradleKotlinUpgradeVersion(componentName, fixVersion, issue);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public MavenInspection() {
public PsiElementVisitor buildVisitor(@NotNull final ProblemsHolder holder, boolean isOnTheFly) {
return new XmlElementVisitor() {
@Override
public void visitXmlTag(XmlTag element) {
public void visitXmlTag(@NotNull XmlTag element) {
if (isDependencyOrPlugin(element)) {
MavenInspection.this.visitElement(holder, element, isOnTheFly);
}
Expand Down Expand Up @@ -99,7 +99,7 @@ String createComponentName(PsiElement element) {
}

@Override
UpgradeVersion getUpgradeVersion(String componentName, String fixVersion, Collection<String> issue) {
UpgradeVersion getUpgradeVersion(String componentName, String fixVersion, Collection<String> issue, String descriptorPath) {
return new MavenUpgradeVersion(componentName, fixVersion, issue);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ String createComponentName(PsiElement element) {
}

@Override
UpgradeVersion getUpgradeVersion(String componentName, String fixVersion, Collection<String> issue) {
return new NpmUpgradeVersion(componentName, fixVersion, issue);
UpgradeVersion getUpgradeVersion(String componentName, String fixVersion, Collection<String> issue, String descriptorPath) {
return new NpmUpgradeVersion(componentName, fixVersion, issue, descriptorPath);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ boolean isMatchingScanner(ScannerBase scanner) {
}

@Override
UpgradeVersion getUpgradeVersion(String componentName, String fixVersion, Collection<String> issue) {
return new YarnUpgradeVersion(componentName, fixVersion, issue);
UpgradeVersion getUpgradeVersion(String componentName, String fixVersion, Collection<String> issue, String descriptorPath) {
return new YarnUpgradeVersion(componentName, fixVersion, issue, descriptorPath);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@
import com.intellij.openapi.project.Project;
import com.jfrog.ide.common.go.GoComponentUpdater;
import com.jfrog.ide.idea.utils.GoUtils;
import com.jfrog.ide.idea.utils.Utils;
import org.jetbrains.annotations.NotNull;

import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collection;

/**
Expand All @@ -16,15 +17,18 @@
* @author michaels
*/
public class GoUpgradeVersion extends UpgradeVersion {
private final String descriptorPath;

public GoUpgradeVersion(String componentName, String fixVersion, Collection<String> issue) {
public GoUpgradeVersion(String componentName, String fixVersion, Collection<String> issue, String descriptorPath) {
super(componentName, fixVersion, issue);
this.descriptorPath = descriptorPath;
}

@Override
public void upgradeComponentVersion(@NotNull Project project, @NotNull ProblemDescriptor descriptor) throws IOException {
Path modulePath = Paths.get(descriptorPath).getParent();
String goExec = GoUtils.getGoExeAndSetEnv(env, project);
GoComponentUpdater goComponentUpdater = new GoComponentUpdater(Utils.getProjectBasePath(project), this.log, this.env, goExec);
GoComponentUpdater goComponentUpdater = new GoComponentUpdater(modulePath, this.log, this.env, goExec);
goComponentUpdater.run(componentName, fixVersion);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
import com.intellij.codeInspection.ProblemDescriptor;
import com.intellij.openapi.project.Project;
import com.jfrog.ide.common.npm.NpmComponentUpdater;
import com.jfrog.ide.idea.utils.Utils;
import org.jetbrains.annotations.NotNull;

import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collection;

/**
Expand All @@ -15,14 +16,17 @@
* @author michaels
*/
public class NpmUpgradeVersion extends UpgradeVersion {
private final String descriptorPath;

public NpmUpgradeVersion(String componentName, String fixVersion, Collection<String> issue) {
public NpmUpgradeVersion(String componentName, String fixVersion, Collection<String> issue, String descriptorPath) {
super(componentName, fixVersion, issue);
this.descriptorPath = descriptorPath;
}

@Override
public void upgradeComponentVersion(@NotNull Project project, @NotNull ProblemDescriptor descriptor) throws IOException {
NpmComponentUpdater npmComponentUpdater = new NpmComponentUpdater(Utils.getProjectBasePath(project), this.log, this.env);
Path modulePath = Paths.get(descriptorPath).getParent();
NpmComponentUpdater npmComponentUpdater = new NpmComponentUpdater(modulePath, this.log, this.env);
npmComponentUpdater.run(componentName, fixVersion);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
import com.intellij.codeInsight.intention.HighPriorityAction;
import com.intellij.codeInspection.LocalQuickFix;
import com.intellij.codeInspection.ProblemDescriptor;
import com.intellij.openapi.application.ApplicationManager;
import com.intellij.openapi.progress.ProgressManager;
import com.intellij.openapi.progress.Task;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Iconable;
import com.intellij.util.EnvironmentUtil;
Expand All @@ -22,7 +25,6 @@
* @author michaels
*/
public abstract class UpgradeVersion implements LocalQuickFix, Iconable, HighPriorityAction {

protected String componentName;
protected String fixVersion;
protected String issue;
Expand Down Expand Up @@ -50,11 +52,19 @@ public String getFamilyName() {

@Override
public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
try {
upgradeComponentVersion(project, descriptor);
} catch (Exception e) {
log.warn("Failed while trying to upgrade component version. Error: " + e);
}
Task.Backgroundable scanAndUpdateTask = new Task.Backgroundable(project, "Upgrading dependency...") {
@Override
public void run(@NotNull com.intellij.openapi.progress.ProgressIndicator indicator) {
try {
upgradeComponentVersion(project, descriptor);
ApplicationManager.getApplication().invokeLater(() -> descriptor.getPsiElement().getContainingFile().getVirtualFile().refresh(false, false));
log.info("Upgraded " + componentName + " to version " + fixVersion + " successfully.");
} catch (Exception e) {
log.error("Failed while trying to upgrade component " + componentName + " to version " + fixVersion + ". Error: " + e);
}
}
};
ProgressManager.getInstance().run(scanAndUpdateTask);
}

abstract public void upgradeComponentVersion(@NotNull Project project, @NotNull ProblemDescriptor descriptor) throws IOException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
import com.intellij.codeInspection.ProblemDescriptor;
import com.intellij.openapi.project.Project;
import com.jfrog.ide.common.yarn.YarnComponentUpdater;
import com.jfrog.ide.idea.utils.Utils;
import org.jetbrains.annotations.NotNull;

import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collection;

/**
Expand All @@ -15,14 +16,17 @@
* @author michaels
*/
public class YarnUpgradeVersion extends UpgradeVersion {
private final String descriptorPath;

public YarnUpgradeVersion(String componentName, String fixVersion, Collection<String> issue) {
public YarnUpgradeVersion(String componentName, String fixVersion, Collection<String> issue, String descriptorPath) {
super(componentName, fixVersion, issue);
this.descriptorPath = descriptorPath;
}

@Override
public void upgradeComponentVersion(@NotNull Project project, @NotNull ProblemDescriptor descriptor) throws IOException {
YarnComponentUpdater yarnComponentUpdater = new YarnComponentUpdater(Utils.getProjectBasePath(project), this.log, this.env);
Path modulePath = Paths.get(descriptorPath).getParent();
YarnComponentUpdater yarnComponentUpdater = new YarnComponentUpdater(modulePath, this.log, this.env);
yarnComponentUpdater.run(componentName, fixVersion);
}
}
5 changes: 4 additions & 1 deletion src/main/java/com/jfrog/ide/idea/scan/ScannerBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,10 @@ private void visitDepTreeNode(Map<String, DependencyNode> dependencies, DepTree
}
DependencyNode existingDep = addedDeps.get(descriptorPath).get(compId);
if (existingDep != null) {
existingDep.setIndirect(indirect);
// If this dependency has any direct path, then it's direct
if (existingDep.isIndirect() && !indirect) {
existingDep.setIndirect(false);
}
continue;
}
// Each dependency might be a child of more than one descriptor, but DependencyNode is a tree node, so it can have only one parent.
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/jfrog/ide/idea/ui/CiComponentsTree.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ private void populateTree(DependencyTree root) {
}

public void addOnProjectChangeListener(MessageBusConnection busConnection) {
busConnection.subscribe(ProjectEvents.ON_SCAN_CI_CHANGE, this::applyFilters);
busConnection.subscribe(ProjectEvents.ON_SCAN_CI_CHANGE, (ProjectEvents) this::applyFilters);
}

private void appendProjectWhenReady(DependencyTree filteredRoot) {
Expand Down

0 comments on commit 0194daa

Please sign in to comment.