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

Fix updating conflicting project #11111

Merged
merged 7 commits into from
Oct 10, 2024
Merged

Fix updating conflicting project #11111

merged 7 commits into from
Oct 10, 2024

Conversation

MrFlashAccount
Copy link
Contributor

@MrFlashAccount MrFlashAccount commented Sep 17, 2024

Pull Request Description

This PR fixes a bug, when uploading a duplicate project to local backend threw a crash on windows.

https://github.com/user-attachments/assets/fe1bd7a7-d840-4bb1-b3f6-e0beec70fcd4
I found a bug when we upload a project to a remote backend, it throws a 500 error. Notified @PabloBuchu about that.

Also, @hubertp proved that the ProjectManager always duplicates a project no matter which option we choose(either "update" or "rename“). So I removed the "update" button if we upload a project locally

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    TypeScript,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • Unit tests have been written where possible.

Copy link
Contributor

@somebody1234 somebody1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CR ✅

@jdunkerley
Copy link
Member

Tested on my Windows box - neither the Update or Rename buttons appear to work.
Clicking cancel works.

@PabloBuchu
Copy link
Contributor

@MrFlashAccount its another PR without any description nor issue. If the bug was reported with discord and there is no issue please create it yourself (its enough to copy paste from discord or insert screenshot and attach to the iteration) or ping me to provide it to you.

@MrFlashAccount MrFlashAccount marked this pull request as draft September 19, 2024 13:59
@PabloBuchu PabloBuchu self-assigned this Sep 24, 2024
@MrFlashAccount MrFlashAccount marked this pull request as ready for review September 24, 2024 14:34
Copy link
Member

@jdunkerley jdunkerley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't remove the functionality from local.
Lets work with @4e6 and @hubertp and get the behavior correct.

Cloud and Local should be the same.

  • If no conflict imported named as in the enso-archive.
  • If conflict, dialog should be shown.
    • If update then replace existing.
    • If rename then add (n) to make unique.

And this should be the case regardless of how the project was "imported" - button, drag drop, double click in OS.

@MrFlashAccount
Copy link
Contributor Author

@jdunkerley reverted the update functionality back, created an issue: #11166

@PabloBuchu PabloBuchu added the CI: Ready to merge This PR is eligible for automatic merge label Oct 8, 2024
@mergify mergify bot merged commit 239a680 into develop Oct 10, 2024
36 checks passed
@mergify mergify bot deleted the wip/sergeigarin/fix-upoad branch October 10, 2024 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--bug Type: bug CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge g-dashboard
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants