Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Project manager should update the asset if they have the same name #11166

Closed
MrFlashAccount opened this issue Sep 25, 2024 · 5 comments
Closed

Comments

@MrFlashAccount
Copy link
Contributor

Steps to reproduce:

  1. Open the dashboard, switch to local mode.
  2. Upload a project with already existing name
  3. On a duplicate dialog, select the "Update" button
  4. Newly uploaded project should replace the old one

More context: #11111 (review)

@4e6
Copy link
Contributor

4e6 commented Sep 25, 2024

Checking the current state of develop on Linux

  1. Open the dashboard, switch to local mode.
  2. Upload a project with already existing name
  3. On a duplicate dialog, select the "Update" button
  4. Newly uploaded project should replace the old one

When importing the project with the same name:
✔️ the Rename button works correctly
❎ the Update button creates a duplicate project with the same name instead of replacing the project.

looking at the logic, I suppose that it was not designed to replace the existing project, but to create a duplicate one:

/** Upload the project from a bundle. */
export async function uploadBundle(
bundle: stream.Readable,
directory?: string | null,
name: string | null = null,
) {
directory ??= getProjectsDirectory()
logger.log(`Uploading project from bundle${name != null ? ` as '${name}'` : ''}.`)
const targetPath = generateDirectoryName(name ?? 'Project', directory)

it even deals with the directory name clashes:

/** Generate a name for a project using given base string. A suffix is added if there is a
* collision.
*
* For example `Name` will become `Name_1` if there's already a directory named `Name`.
* If given a name like `Name_1` it will become `Name_2` if there is already a directory named
* `Name_1`. If a path containing multiple components is given, only the last component is used
* for the name. */
export function generateDirectoryName(name: string, directory = getProjectsDirectory()): string {

@4e6 4e6 added -gui and removed triage labels Sep 25, 2024
@4e6 4e6 removed their assignment Sep 25, 2024
@4e6
Copy link
Contributor

4e6 commented Sep 25, 2024

Also, a comment about the renaming logic of generateDirectoryName function

If given a name like Name_1 it will become Name_2 if there is already a directory named Name_1

I used to name the projects according to the issue. For example, to test this issue I created a project Issue_11166, and the first time I tested, I did not expect and completely missed the new Issue_11167 and Issue_11168 directories that were created when I imported the project twice.

Perhaps this naming logic is acceptable if we don't expect our users to use numbers to distinguish their projects.

@somebody1234
Copy link
Contributor

well, it is a good point to raise, as you might run into this issue for dates as well

@4e6
Copy link
Contributor

4e6 commented Sep 25, 2024

AFAIK the Cloud adds (1) when duplicating the project. Although I'm not sure how different filesystems would handle the brackets

@somebody1234
Copy link
Contributor

yeah. we use brackets for names in metadata, not sure what we should do for directory names. i think the intention of using plain old _ was to avoid risking breakage, although we know for sure that windows and linux (and almost certainly macos) handle it fine. so perhaps we should change to (n)

@JaroslavTulach JaroslavTulach changed the title Project manajer should update the asset if they have the same name Project manager should update the asset if they have the same name Oct 8, 2024
@MrFlashAccount MrFlashAccount self-assigned this Oct 14, 2024
@MrFlashAccount MrFlashAccount moved this from ❓New to 👁️ Code review in Issues Board Oct 14, 2024
@github-project-automation github-project-automation bot moved this from 👁️ Code review to 🟢 Accepted in Issues Board Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🟢 Accepted
Development

No branches or pull requests

5 participants