Assistance Needed: Resolving Issue #3222 in MIT App Inventor

Hi everyone,

I'm a new contributor to MIT App Inventor, and I'm currently trying to resolve Issue #3222. I'm facing a problem with the code I wrote and would appreciate any guidance or suggestions on why it's not working as expected.

Problem

When empty folders are selected in the project explorer and sent to the trash, they still appear in the project list after the page is reloaded. The expected behavior is that these empty folders should be completely removed.

Proposed Solution

I believe the solution lies in ensuring that empty folders are correctly removed both from the project list and the trash folder. This likely involves updating the FolderManager.java file to handle the deletion of empty folders properly.

Could anyone help me identify what might be going wrong or provide feedback on this approach?.

// FolderManager.java

public void moveItemsToFolder(List<Project> projects, List<ProjectFolder> folders, ProjectFolder destination) {
  LOG.info("Moving projects count " + projects.size() + " to " + destination.getName());
  for (Project project : projects) {
    LOG.info("Moving project " + project.getProjectName()  + " from "
        + project.getHomeFolder().getName() + " to " + destination.getName());
    destination.addProject(project);
  }
  for (ProjectFolder folder : folders) {
    LOG.info("Moving folder " + folder.getName()  + " from " + folder.getParentFolder().getName()
        + " to " + destination.getName());
    destination.addChildFolder(folder);
    // Check if the folder is empty and delete it if it is
    if (folder.getProjects().isEmpty() && folder.getChildFolders().isEmpty()) {
      folder.getParentFolder().removeChildFolder(folder);
    }
  }
  saveAllFolders();
  fireFoldersChanged();
}

public void loadFolders() {
  String foldersAsString = Ode.getUserSettings()
      .getSettings(SettingsConstants.USER_GENERAL_SETTINGS)
      .getPropertyValue(SettingsConstants.FOLDERS);
  foldersLoaded = true;

  if (foldersAsString.isEmpty()) {
    LOG.info("Initialize folders");
    initializeFolders();
    fireFoldersLoaded();
    return;
  }

  JSONObject folderJSON = JSONParser.parse(foldersAsString).isObject();
  if (folderJSON.get(FolderJSONKeys.PROJECTS).isArray().size() == 0
      && folderJSON.get(FolderJSONKeys.CHILD_FOLDERS).isArray().size() == 0) {
    LOG.info("Global folder is empty");
    initializeFolders();
    fireFoldersLoaded();
    return;
  }

  LOG.info("folderJSON - " + folderJSON);
  globalFolder = uiFactory.createProjectFolder(folderJSON, null);
  LOG.info("Creating Trash Folder");
  trashFolder = globalFolder.getChildFolder(FolderJSONKeys.TRASH_FOLDER);
  checkForUnassignedProjects();
  removeEmptyFolders(trashFolder);
  fireFoldersLoaded();
}

private void removeEmptyFolders(ProjectFolder folder) {
  List<ProjectFolder> emptyFolders = new ArrayList<>();
  for (ProjectFolder childFolder : folder.getChildFolders()) {
    if (childFolder.getProjects().isEmpty() && childFolder.getChildFolders().isEmpty()) {
      emptyFolders.add(childFolder);
    } else {
      removeEmptyFolders(childFolder);
    }
  }
  for (ProjectFolder emptyFolder : emptyFolders) {
    folder.removeChildFolder(emptyFolder);
  }
}```

Why This Code?
* moveItemsToFolder: Ensures that empty folders are removed when moved to the trash.
* loadFolders: Ensures that empty folders are not reloaded from the trash.
* removeEmptyFolders: Recursively removes empty folders from their parent folder.

The approach sounds reasonable. Is there a specific bug we are looking to resolve in the proposed code?

The above code does not resolve the issue#3222. Could you please review the code I shared above? If there’s anything I need to correct or improve, I would greatly appreciate your guidance.

Here are the parameters of the current system:

  • When projects are moved to trash, they are stripped from their folders. You will notice that all projects in trash are in a flat list, no matter where they were in the folder hierarchy when trashed.
  • When a folder is selected and the trash button clicked, the system pulls all the projects from the selected folder and recursive subfolders, strips the projects out of their folders, and moves them to trash.
  • The active project list needs to be able to handle empty folders because folders are empty when they are created. Moreover, many users create their folder hierarchy first and then move projects into it. They may not want the folder deleted if they temporarily move all projects out of it to other active folders.

I designed the trash system to handle projects and not folders because a trash system that handles folders has approximately a zillion edge cases. If you experiment with different ways to trash and restore files/folders in Windows or MacOS, you'll see what I mean. We don't have the sophisticated drag-and-drop mechanics that those operating systems have either.

So currently, the projects are pulled out of their folders when trashed, and the system has no way to distinguish an empty folder that is supposed to be empty and an empty folder that should be deleted.

Now, the top of our hierarchy features the *global* folder and a hidden folder *trash*. The code you posted looks like it's the right idea. Does this work when moving a project from one folder to another in the active projects view? There should be code that checks for *Trash* specifically and handles it as a special case.

ETA: You ONLY want to delete folders when they are both sent to trash and empty after all the projects are trashed. Folders that are empty from an ordinary folder-to-folder move should not be deleted for the reasons above. This may be as straightforward as checking for *Trash* as a special destination in the code you posted.

1 Like

As additional information, I think the proper deletion of trashed folders did use to work. I vaguely remember that there was a bug where empty folders in the active project list got removed when the list was reloaded. I assume that when this bug was fixed, the trashed folders were broken.

1 Like

Hi everyone,

Apologies for the delayed response; I’ve been busy with my semester exams. I’ve been analyzing the issue related to folders remaining visible in the project list after being sent to trash, and I would appreciate your guidance on my approach so far.

After investigating, I noticed that when folders are moved to trash, no API call is triggered. In contrast, projects have a working API call (moveToTrash) to handle this functionality. I traced this to the ProjectServiceImpl.java file and identified the relevant methods. While analyzing the code, I realized that the deleteFolder functionality isn’t working as expected when folders are deleted.

To address this, I made modifications in DeleteAction.java to include a call to the deleteFolder method for each selected folder. Here’s the code I updated:

DeleteAction.java

@Override
  public void execute() {
    Ode.getInstance().getEditorManager().saveDirtyEditors(new Command() {
      @Override
      public void execute() {
        if (Ode.getInstance().getCurrentView() == Ode.PROJECTS) {
          List<Project> selectedProjects = ProjectListBox.getProjectListBox().getProjectList().getSelectedProjects();
          List<ProjectFolder> selectedFolders = ProjectListBox.getProjectListBox().getProjectList().getSelectedFolders();
          if (selectedProjects.size() > 0 || selectedFolders.size() > 0) {
            List<Project> projectsToDelete = selectedProjects;
            for (ProjectFolder f : selectedFolders) {
              projectsToDelete.addAll(f.getNestedProjects());
              deleteFolder(f); // Call deleteFolder for each selected folder
            }
            if (deleteConfirmation(projectsToDelete)) {
              for (Project project : projectsToDelete) {
                project.moveToTrash();
              }
              for (ProjectFolder f : selectedFolders) {
                f.getParentFolder().removeChildFolder(f);
              }
            }
          } else {
            ErrorReporter.reportInfo(MESSAGES.noProjectSelectedForDelete());
          }
        } else {
          List<Project> selectedProjects = new ArrayList<Project>();
          Project currentProject = Ode.getInstance().getProjectManager().getProject(Ode.getInstance().getCurrentYoungAndroidProjectId());
          selectedProjects.add(currentProject);
          if (deleteConfirmation(selectedProjects)) {
            currentProject.moveToTrash();
          }
        }
        Ode.getInstance().switchToProjectsView();
      }
    });
  }

private void deleteFolder(ProjectFolder folder) {
    long projectId = folder.getProject(); // Ensure you have a method to get the project ID
    Ode.getInstance().getProjectService().deleteFolder(
        Ode.getInstance().getSessionId(),
        projectId,
        folder.getName(),
        new AsyncCallback<Long>() {
            @Override
            public void onFailure(Throwable caught) {
                ErrorReporter.reportError("Failed to delete folder: " + folder.getName());
            }

            @Override
            public void onSuccess(Long result) {
                // Folder deleted successfully
            }
        }
    );
}

ProjectFolder.java

public long getProject() {
    return getCurrentProjectID();
}

Now, when folders are moved to trash, the API call is successfully triggered (as seen in the picture attached).

As a beginner, I find this issue quite challenging, and I’m unsure if my approach is the right direction. Specifically:

  1. Should we implement a trashing system for folders as well, or is there a better way to handle this issue?
  2. Currently, empty folders are still visible after being sent to trash. One potential approach could be to add a placeholder project for empty folders. This could help differentiate between folders that are intentionally empty and those that should be deleted. During my analysis, I noticed that when a folder containing projects is moved to trash, the folder does not reappear after reloading the page, indicating a mechanism that only fetches projects, not folders. While this solution might temporarily address the issue, I feel it’s not ideal. However, it would avoid the need for implementing a complete trashing mechanism for folders similar to projects.

@Susan_Lane (or any One), I’d appreciate your guidance on how to approach this issue effectively. I want to solve this but recognize it may take me some time as I’m new to the codebase.

Thank you for your patience and help!

This looks like a good approach, but how to get all forms of deletion working in some kind of consistent intuitive way isn't obvious.

I think you should put up a PR, mark it as draft, and request me as a reviewer. Do you need some details on how to do that?

1 Like

Thank you for the feedback, I agree that ensuring consistency in all forms of deletion is a bit challenging, but I’ll go ahead and put up a draft PR as suggested. I’ll mark it as a draft and request you as a reviewer.

I probably wasn't clear that the way to make the behavior consistent isn't clear to me either, which is why I wanted you to put up a draft PR. That allows me to pull down the change for myself and see how it behaves.

1 Like

@Susan_Lane please review my PR, here is the PR Link

I'm looking at it. Thanks!

1 Like

@Susan_Lane Could you please review this PR as well?
i don't know why all checks are failed please guide me

1 Like