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 duplicating requests #878

Merged
merged 3 commits into from
Jul 31, 2023
Merged

Fix duplicating requests #878

merged 3 commits into from
Jul 31, 2023

Conversation

akurinnoy
Copy link
Contributor

@akurinnoy akurinnoy commented Jul 28, 2023

What does this PR do?

This pull request fixes a bug that caused the Dashboard to send duplicate requests when creating or starting workspaces.

What issues does this PR fix or reference?

fixes eclipse-che/che#22326
fixes eclipse-che/che#22323

Is it tested? How?

  1. Deploy Eclipse Che
  2. Update CheCluster CR to use quay.io/eclipse/che-dashboard:pr-878 as the dashboard image:
    spec:
      components:
        dashboard:
          deployment:
            containers:
              - image: quay.io/eclipse/che-dashboard:pr-878
  3. Create a workspace either from a devfile or a sample.
  4. Open Developer tools->Network, and make sure there are no requests with 409 Conflict status.
  5. Open the Dashboard workspaces list page and make sure there is only one new workspace.

Signed-off-by: Oleksii Kurinnyi <okurinny@redhat.com>
Signed-off-by: Oleksii Kurinnyi <okurinny@redhat.com>
@akurinnoy akurinnoy self-assigned this Jul 28, 2023
@che-bot
Copy link
Contributor

che-bot commented Jul 28, 2023

Click here to review and test in web IDE: Contribute

@github-actions
Copy link

Docker image build succeeded: quay.io/eclipse/che-dashboard:pr-878

@codecov
Copy link

codecov bot commented Jul 28, 2023

Codecov Report

Merging #878 (4e56d2a) into main (5c06ca2) will increase coverage by 0.01%.
Report is 3 commits behind head on main.
The diff coverage is 88.33%.

@@            Coverage Diff             @@
##             main     #878      +/-   ##
==========================================
+ Coverage   81.53%   81.54%   +0.01%     
==========================================
  Files         346      347       +1     
  Lines       35750    35962     +212     
  Branches     2224     2244      +20     
==========================================
+ Hits        29147    29325     +178     
- Misses       6581     6615      +34     
  Partials       22       22              
Flag Coverage Δ
unittests 81.54% <88.33%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...rontend/src/components/WorkspaceProgress/index.tsx 94.27% <84.31%> (-2.57%) ⬇️
.../src/components/WorkspaceProgress/Wizard/index.tsx 90.40% <90.40%> (ø)

... and 1 file with indirect coverage changes

@ibuziuk
Copy link
Member

ibuziuk commented Jul 31, 2023

@akurinnoy please, backport to 7.72.x for 3.8 once merged in main

@olexii4
Copy link
Contributor

olexii4 commented Jul 31, 2023

@akurinnoy I tested it. I clicked on a sample and
I see 3 requests {che-server}/dashboard/api/server-config instead of one while creating a new workspace.

Знімок екрана 2023-07-31 о 13 50 53

Could you fix it too?

@openshift-ci openshift-ci bot removed the lgtm label Jul 31, 2023
@akurinnoy
Copy link
Contributor Author

Could you fix it too?

@olexii4 I believe that's another issue not related to this one.

@akurinnoy
Copy link
Contributor Author

@olexii4 FYI the fixup I added (4e56d2a) decreases the number of re-renderings of WorkspaceProgress component.

@github-actions
Copy link

Docker image build succeeded: quay.io/eclipse/che-dashboard:pr-878

Copy link
Contributor

@olexii4 olexii4 left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-ci
Copy link

openshift-ci bot commented Jul 31, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: akurinnoy, ibuziuk, olexii4

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@akurinnoy akurinnoy merged commit 9ebccaf into main Jul 31, 2023
@akurinnoy akurinnoy deleted the fix-duplicate-requests branch July 31, 2023 13:24
akurinnoy added a commit that referenced this pull request Jul 31, 2023
akurinnoy added a commit that referenced this pull request Aug 2, 2023
Fix duplicating requests (#878)
fix: workspace condition messages (#884)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UD dashboard send the same requests double times two workspaces are created instead of one
4 participants