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.

Sign up for GitHub

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

Jump to bottom

fix: App card is created even when app import fails and no action can be performed on this app card #18034

Merged
merged 3 commits into from Nov 25, 2022
Merged

fix: App card is created even when app import fails and no action can be performed on this app card #18034

merged 3 commits into from Nov 25, 2022

Conversation

AnaghHegde
Copy link
Member

@AnaghHegde AnaghHegde commented Nov 1, 2022 •
edited

Description

App card is created with no timestamp details when app import fails. This app card is a placeholder and performing any action like edit or launch throws a resource not found error.

Fixes #10490

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Manual
  • JUnit

Checklist:

Dev activity

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • PR is being merged under a feature flag

QA activity:

  • Test plan has been approved by relevant developers
  • Test plan has been peer reviewed by QA
  • Cypress test cases have been added and approved by either SDET or manual QA
  • Organized project review call with relevant stakeholders after Round 1/2 of QA
  • Added Test Plan Approved label after reveiwing all Cypress test

@AnaghHegde AnaghHegde self-assigned this Nov 1, 2022
@vercel
Copy link

vercel bot commented Nov 1, 2022 •
edited

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
appsmith ✅ Ready (Inspect) Visit Preview Nov 22, 2022 at 4:09AM (UTC)

@github-actions github-actions bot added Bug Something isn't working High This issue blocks a user from building or impacts a lot of users Import-Export-App Issues related to importing and exporting apps Production Team Managers Pod Issues that team managers care about for the security and efficiency of their teams labels Nov 1, 2022
@AnaghHegde
Copy link
Member Author

/ok-to-test sha=49f47eb

trishaanand
trishaanand approved these changes Nov 2, 2022
View reviewed changes
Copy link
Contributor

@trishaanand trishaanand left a comment

Choose a reason for hiding this comment

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

Minor question added.

...ver/src/main/java/com/appsmith/server/solutions/ce/ImportExportApplicationServiceCEImpl.java
@@ -729,6 +729,7 @@ private Mono<Application> importApplicationInWorkspace(String workspaceId,
// Check if the destination org have all the required plugins installed
for (Datasource datasource : importedDatasourceList) {
if (StringUtils.isEmpty(pluginMap.get(datasource.getPluginId()))) {
log.error("Unable to find the plugin ", datasource.getPluginId());
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know when this happens? On the same instance, the plugin ids should remain the same no?

Also, instead of logging this error here, do you know why Mono.error is not logging it? If it is not, I suggest that you check out the Global Exception Handler?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hey, the error is logged but because the onErrorResume is catching this error hence I had to add this condition.

@AnaghHegde
Copy link
Member Author

/ok-to-test sha=0e1457f

@github-actions
Copy link

github-actions bot commented Nov 3, 2022

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3383222694.
Workflow: Appsmith External Integration Test Workflow.
Commit: 0e1457f.
PR: 18034.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=18034&runId=3383222694_1

@github-actions
Copy link

This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected.

@github-actions github-actions bot added the Stale label Nov 10, 2022
@github-actions
Copy link

This PR has been closed because of inactivity.

@github-actions github-actions bot closed this Nov 18, 2022
@AnaghHegde AnaghHegde reopened this Nov 22, 2022
@AnaghHegde
Copy link
Member Author

/ok-to-test sha=910dc61

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3520258302.
Workflow: Appsmith External Integration Test Workflow.
Commit: 910dc61.
PR: 18034.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=18034&runId=3520258302_1

@github-actions github-actions bot removed the Stale label Nov 22, 2022
@AnaghHegde AnaghHegde merged commit a7cb2ce into release Nov 25, 2022
@AnaghHegde AnaghHegde deleted the fix/app-card-import-export-file branch November 25, 2022 06:09
@AnaghHegde AnaghHegde mentioned this pull request Dec 15, 2022
Merged
13 tasks
mohanarpit pushed a commit that referenced this pull request Dec 15, 2022
@AnaghHegde
fix: App deleted when the user creates branch in few scenarios (#18964)
051444f 
## Description

> Continuing from the PR - #18034. The app getting deleted bug was introduced when creating branch was introduced in the linked PR. 
Current PR adds a check to ensure that none of the git flow is affected. 

Fixes - https://app.frontapp.com/inboxes/teams/folders/15585868/unassigned/66043703436

## Type of change

- Bug fix (non-breaking change which fixes an issue)


## How Has This Been Tested?

- Manual

## Checklist:
### Dev activity
- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag


### QA activity:
- [ ] Test plan has been approved by relevant developers
- [ ] Test plan has been peer reviewed by QA
- [ ] Cypress test cases have been added and approved by either SDET or manual QA
- [ ] Organized project review call with relevant stakeholders after Round 1/2 of QA
- [ ] Added Test Plan Approved label after reveiwing all Cypress test
AnaghHegde added a commit that referenced this pull request Dec 15, 2022
@AnaghHegde
fix: App deleted when the user creates branch in few scenarios (#18964)
9ae76f1 
## Description

> Continuing from the PR - #18034. The app getting deleted bug was introduced when creating branch was introduced in the linked PR. 
Current PR adds a check to ensure that none of the git flow is affected. 

Fixes - https://app.frontapp.com/inboxes/teams/folders/15585868/unassigned/66043703436

## Type of change

- Bug fix (non-breaking change which fixes an issue)


## How Has This Been Tested?

- Manual

## Checklist:
### Dev activity
- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag


### QA activity:
- [ ] Test plan has been approved by relevant developers
- [ ] Test plan has been peer reviewed by QA
- [ ] Cypress test cases have been added and approved by either SDET or manual QA
- [ ] Organized project review call with relevant stakeholders after Round 1/2 of QA
- [ ] Added Test Plan Approved label after reveiwing all Cypress test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trishaanand trishaanand trishaanand approved these changes

@sidhantgoel sidhantgoel Awaiting requested review from sidhantgoel

Assignees

@AnaghHegde AnaghHegde

Labels
Bug Something isn't working High This issue blocks a user from building or impacts a lot of users Import-Export-App Issues related to importing and exporting apps Production Team Managers Pod Issues that team managers care about for the security and efficiency of their teams
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]-[224]:App card is created even when app import fails and no action can be performed on this app card
2 participants
@AnaghHegde @trishaanand