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

Default tab fixed for Library page - issue #3871 #3897

Merged
merged 8 commits into from
May 24, 2024
Merged

Conversation

friyad
Copy link
Contributor

@friyad friyad commented May 21, 2024

Description

When a user clicks on Library they see Blueprints route as default. But it should be the Team Library route. That was the problem and by this pull request, this issue has been solved.

Related Issue(s)

Issue: #3871

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production

Labels

  • Includes a DB migration? -> add the area:migration label

When a user clicks on Library they see Blueprints route as default. But it should be Team Library route. That was the problem and by this commit this issue has been solved
Copy link
Contributor

@joepavitt joepavitt left a comment

Choose a reason for hiding this comment

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

/home/runner/work/flowfuse/flowfuse/frontend/src/pages/team/Library/index.vue
42:18 error Unexpected trailing comma comma-dangle
/home/runner/work/flowfuse/flowfuse/frontend/src/pages/team/Library/routes.js
6:77 error Unexpected trailing comma comma-dangle

Please fix the errors/warnings reported in the tests

Copy link
Contributor

@cstns cstns left a comment

Choose a reason for hiding this comment

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

I have a feeling that some e2e tests will also fail after fixing the lint errors due to the library's default page being changed

frontend/src/pages/team/Library/routes.js Outdated Show resolved Hide resolved
@joepavitt
Copy link
Contributor

As @cstns predicated - the E2E tests are failing given that teh test checks the URL is /blueprints- @friyad can you update and check they're passing please?

@friyad
Copy link
Contributor Author

friyad commented May 22, 2024

Can anyone please give me some hints on how can I solve it? I am new at testing this type of large codebase. Though I haven't read the full codebase yet. On the test result/error, it is indicating /test/e2e/frontend/cypress/tests/team/library.spec.js in this test file.

    beforeEach(() => {
        cy.login('alice', 'aaPassword')
        cy.home()
    })

    describe('Blueprints', () => {
        it('should load the Library page and display the unavailable feature banner for the Blueprints tab', () => {
            cy.visit('team/ateam/library')

            cy.get('[data-el="page-name"]').contains('Library')
            cy.contains('Shared repository to store common flows and nodes.')
            cy.contains('No Blueprints Available')
            cy.contains('This is a FlowFuse Premium feature. Please upgrade your instance of FlowFuse in order to use it.')
        })
    })
    describe('Team Library', () => {
        it('should load the Library page and display the unavailable feature banner for Team Library tab', () => {
            cy.visit('team/ateam/library')

            cy.get('[data-el="ff-tab"]').contains('Team Library').click()
            cy.contains('Create your own Team Library')
            cy.contains('This is a FlowFuse Premium feature. Please upgrade your instance of FlowFuse in order to use it.')
        })
    })
})

do I need to change the cy.get(...)?

@cstns
Copy link
Contributor

cstns commented May 22, 2024

You can find a detailed a how-to run the cypress suite in https://flowfuse.com/handbook/development/frontend/testing/#e2e-integration-tests

do I need to change the cy.get(...)?

In a nutshell, no, not only that because the cypress suite visit's the library page and after the redirect it expects to find elements of the blueprint page because that was the default up to now and fails because it can't find them.

cy.get('[data-el="page-name"]') pinpoints elements on the page that have the data-el attribute equal to "page-name". They're just locators, similar to classes but more precise.

I'm sure you'll get the gist of it once you run the e2e tests, if not, let me know and I'll help you out

@friyad
Copy link
Contributor Author

friyad commented May 22, 2024

Hello @cstns,
I have run this test locally as per the link you provided. But, I am getting this test error:

 1) FlowForge - Library
       Blueprints tab
         allows users to create new Blueprints if they don't have  any:
     CypressError: Timed out retrying after 5000ms: `cy.wait()` timed out waiting `5000ms` for the 1st request to the route: `getBlueprints`. No request ever occurred.

https://on.cypress.io/wait
      at cypressErr (http://localhost:3002/__cypress/runner/cypress_runner.js:75257:18)
      at Object.errByPath (http://localhost:3002/__cypress/runner/cypress_runner.js:75312:10)
      at checkForXhr (http://localhost:3002/__cypress/runner/cypress_runner.js:135593:84)
      at <unknown> (http://localhost:3002/__cypress/runner/cypress_runner.js:135619:28)
      at tryCatcher (http://localhost:3002/__cypress/runner/cypress_runner.js:1807:23)
      at Promise.attempt.Promise.try (http://localhost:3002/__cypress/runner/cypress_runner.js:4315:29)
      at whenStable (http://localhost:3002/__cypress/runner/cypress_runner.js:144032:68)
      at <unknown> (http://localhost:3002/__cypress/runner/cypress_runner.js:143973:14)
      at tryCatcher (http://localhost:3002/__cypress/runner/cypress_runner.js:1807:23)
      at Promise._settlePromiseFromHandler (http://localhost:3002/__cypress/runner/cypress_runner.js:1519:31)
      at Promise._settlePromise (http://localhost:3002/__cypress/runner/cypress_runner.js:1576:18)
      at Promise._settlePromise0 (http://localhost:3002/__cypress/runner/cypress_runner.js:1621:10)
      at Promise._settlePromises (http://localhost:3002/__cypress/runner/cypress_runner.js:1701:18)
      at Promise._fulfill (http://localhost:3002/__cypress/runner/cypress_runner.js:1645:18)
      at <unknown> (http://localhost:3002/__cypress/runner/cypress_runner.js:5450:46)
  From Your Spec Code:
      at Context.eval (webpack:///../test/e2e/frontend/cypress/tests-ee/team/library.spec.js:47:15)

  2) FlowForge - Library
       Blueprints tab
         allows users to select a predefined blueprint and create an instance:
     AssertionError: Timed out retrying after 4000ms: Expected to find element: `[data-el="5678"]`, but never found it.
      at Context.eval (webpack:///../test/e2e/frontend/cypress/tests-ee/team/library.spec.js:102:15)

  3) FlowForge - Library
       Blueprints tab
         allows users to preview predefined blueprints:
     AssertionError: Timed out retrying after 4000ms: Expected to find element: `[data-el="flow-view-dialog"]`, but never found it.
      at Context.eval (webpack:///../test/e2e/frontend/cypress/tests-ee/team/library.spec.js:113:51)

As I am still new to tests, I think it would be better if you could help me write these tests

@cstns
Copy link
Contributor

cstns commented May 22, 2024

I'll be more than happy to help! I'll make the changes on this PR and leave comments on the changes

@cstns
Copy link
Contributor

cstns commented May 22, 2024

I was incapable of updating your pr and inadvertently created a new one. I marked it as draft so it won't get merged. Look over the comments and move the necessary changes here.

#3915

@cstns
Copy link
Contributor

cstns commented May 23, 2024

@friyad, there are a couple of linting errors that prevent the e2e tests from completing successfully. I suspect they are the last thing that would need solving before accepting the PR

Copy link

codecov bot commented May 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.28%. Comparing base (45b1a1c) to head (49f6c18).

Current head 49f6c18 differs from pull request most recent head bcb02ff

Please upload reports for the commit bcb02ff to get more accurate results.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3897   +/-   ##
=======================================
  Coverage   79.28%   79.28%           
=======================================
  Files         281      281           
  Lines       12755    12755           
  Branches     2844     2844           
=======================================
  Hits        10113    10113           
  Misses       2642     2642           
Flag Coverage Δ
backend 79.28% <ø> (ø)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@friyad
Copy link
Contributor Author

friyad commented May 24, 2024

@cstns Thanks for noticing the error. I solved the lint errors. I didn't change any files there. I think these errors are coming after the merge. I also ran the e2e test. But, I am getting this error. I need your help with this:

 1) FlowForge - Devices - With Billing
       offers correct options in snapshot table kebab menu:

      AssertionError: Timed out retrying after 4000ms: Not enough elements found. Found '2', expected '5'.
      + expected - actual

      -2
      +5

      at Context.eval (webpack:///../test/e2e/frontend/cypress/tests-ee/devices/snapshots.spec.js:124:101)

  2) FlowForge - Devices - With Billing
       provides functionality to view a snapshot:
     CypressError: Timed out retrying after 4050ms: `cy.click()` failed because this element:

`<li class="ff-list-item ff-list-item--danger disabled">...</li>`

has CSS `pointer-events: none`

`pointer-events: none` prevents user mouse interaction.

Fix this problem, or use {force: true} to disable error checking.

https://on.cypress.io/element-cannot-be-interacted-with
      at ensureElDoesNotHaveCSS (http://localhost:3002/__cypress/runner/cypress_runner.js:111888:66)
      at ensureDescendents (http://localhost:3002/__cypress/runner/cypress_runner.js:111996:5)
      at ensureDescendentsAndScroll (http://localhost:3002/__cypress/runner/cypress_runner.js:112004:14)
      at ensureElIsNotCovered (http://localhost:3002/__cypress/runner/cypress_runner.js:112135:5)
      at runAllChecks (http://localhost:3002/__cypress/runner/cypress_runner.js:112334:52)
      at retryActionability (http://localhost:3002/__cypress/runner/cypress_runner.js:112371:16)
      at tryCatcher (http://localhost:3002/__cypress/runner/cypress_runner.js:1807:23)
      at Promise.attempt.Promise.try (http://localhost:3002/__cypress/runner/cypress_runner.js:4315:29)
      at whenStable (http://localhost:3002/__cypress/runner/cypress_runner.js:144032:68)
      at <unknown> (http://localhost:3002/__cypress/runner/cypress_runner.js:143973:14)
      at tryCatcher (http://localhost:3002/__cypress/runner/cypress_runner.js:1807:23)
      at Promise._settlePromiseFromHandler (http://localhost:3002/__cypress/runner/cypress_runner.js:1519:31)
      at Promise._settlePromise (http://localhost:3002/__cypress/runner/cypress_runner.js:1576:18)
      at Promise._settlePromise0 (http://localhost:3002/__cypress/runner/cypress_runner.js:1621:10)
      at Promise._settlePromises (http://localhost:3002/__cypress/runner/cypress_runner.js:1701:18)
      at Promise._fulfill (http://localhost:3002/__cypress/runner/cypress_runner.js:1645:18)
      at <unknown> (http://localhost:3002/__cypress/runner/cypress_runner.js:5450:46)
  From Your Spec Code:
      at Context.eval (webpack:///../test/e2e/frontend/cypress/tests-ee/devices/snapshots.spec.js:155:123)

@cstns
Copy link
Contributor

cstns commented May 24, 2024

Please push the changes with the merge included, I need to see the code

@cstns cstns requested a review from joepavitt May 24, 2024 14:03
@cstns cstns dismissed joepavitt’s stale review May 24, 2024 14:05

linting issues are now resolved

@cstns
Copy link
Contributor

cstns commented May 24, 2024

Looks good, happy to approve!

@cstns cstns merged commit 2f654bf into FlowFuse:main May 24, 2024
11 of 12 checks passed
@friyad
Copy link
Contributor Author

friyad commented May 24, 2024

Thanks a lot @cstns, @joepavitt and @ZJvandeWeg. It was my first open-source contribution. Looking forward to contributing more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants