search

FlowFuse / flowfuse

Build bespoke, flexible, and resilient manufacturing low-code applications with FlowFuse and Node-RED
https://flowfuse.com
Other
269 stars 63 forks source link

users without accounts should be redirected to the blueprint landing page after registration #4020

Closed cstns closed 3 months ago

cstns commented 3 months ago

Description

Adds support for unregistered users to successfully follow the instance creation process when accessing a blueprint landing page

Adds support for

Related Issue(s)

closes #3999 depends on #4043 depends on #4045 #4050

Checklist

Labels

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 78.72%. Comparing base (6a3313a) to head (a1c94a0).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## smtp-affair #4020 +/- ## ============================================ Coverage 78.72% 78.72% ============================================ Files 284 284 Lines 13009 13009 Branches 2897 2897 ============================================ Hits 10241 10241 Misses 2768 2768 ``` | [Flag](https://app.codecov.io/gh/FlowFuse/flowfuse/pull/4020/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FlowFuse) | Coverage Δ | | |---|---|---| | [backend](https://app.codecov.io/gh/FlowFuse/flowfuse/pull/4020/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FlowFuse) | `78.72% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FlowFuse#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Steve-Mcl commented 3 months ago

@cstns Are these test fails related to this work?

FlowFuse platform admin users
    1) can enable sign up
    2) can customise the content of the "Sign Up" screen

  2 failing

  1) FlowFuse platform admin users
       can enable sign up:
     CypressError: Timed out retrying after 4050ms: `cy.click()` failed because this element is `disabled`:

`<button class="ff-btn transition-fade--color ff-btn--primary" type="button" disabled="" data-action="save-settings">Save se...</button>`

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

https://on.cypress.io/element-cannot-be-interacted-with
      at Object.isNotDisabled (http://localhost:3001/__cypress/runner/cypress_runner.js:145210:58)
      at runAllChecks (http://localhost:3001/__cypress/runner/cypress_runner.js:112282:26)
      at retryActionability (http://localhost:3001/__cypress/runner/cypress_runner.js:112371:16)
      at tryCatcher (http://localhost:3001/__cypress/runner/cypress_runner.js:1807:23)
      at Promise.attempt.Promise.try (http://localhost:3001/__cypress/runner/cypress_runner.js:4315:29)
      at whenStable (http://localhost:3001/__cypress/runner/cypress_runner.js:144032:68)
      at <unknown> (http://localhost:3001/__cypress/runner/cypress_runner.js:143973:14)
      at tryCatcher (http://localhost:3001/__cypress/runner/cypress_runner.js:1807:23)
      at Promise._settlePromiseFromHandler (http://localhost:3001/__cypress/runner/cypress_runner.js:1519:31)
      at Promise._settlePromise (http://localhost:3001/__cypress/runner/cypress_runner.js:1576:18)
      at Promise._settlePromise0 (http://localhost:3001/__cypress/runner/cypress_runner.js:1621:10)
      at Promise._settlePromises (http://localhost:3001/__cypress/runner/cypress_runner.js:1701:18)
      at Promise._fulfill (http://localhost:3001/__cypress/runner/cypress_runner.js:1645:18)
      at <unknown> (http://localhost:3001/__cypress/runner/cypress_runner.js:5450:46)
  From Your Spec Code:
      at Context.eval (webpack:///../test/e2e/frontend/cypress/tests/admin.spec.js:119:48)

  2) FlowFuse platform admin users
       can customise the content of the "Sign Up" screen:
     AssertionError: Timed out retrying after 4000ms: Expected to find element: `[data-el="banner"]`, but never found it.
      at Context.eval (webpack:///../test/e2e/frontend/cypress/tests/admin.spec.js:[139](https://github.com/FlowFuse/flowfuse/actions/runs/9601469621/job/26480109014?pr=4020#step:6:140):11)
Steve-Mcl commented 3 months ago

@cstns I updated this branch (via the GH update button) to get the tests to run again. Hope thats ok?

cstns commented 3 months ago

I'd need to merge https://github.com/FlowFuse/flowfuse/pull/4050 to fix the failing tests. I'll have to address it first and merge the PR once the tests pass. I hope to get it sorted by EOD

Steve-Mcl commented 3 months ago

I'd need to merge #4050 to fix the failing tests. I'll have to address it first and merge the PR once the tests pass. I hope to get it sorted by EOD

ok, np. Please tag me in a comment & refresh the "request review" once ready so that i get notified.

Ta.

joepavitt commented 3 months ago

I hope to get it sorted by EOD

Not clear to me what the order of actions is, this was also due 2 days ago.

cstns commented 3 months ago

The fix that I came up with is to wait for the container logs to print out that the smtp server is up an running. It's not perfect but it works locally and in CI runs. I'd need to spend some additional time on it to create a smtp service for CI runs and sort out other loose ends.

I wouldn't want to hold up this feature on this small quirk any longer. I'm going to create a follow up story for the smtp cleanup to move things along.

cstns commented 3 months ago

created https://github.com/FlowFuse/flowfuse/issues/4075

cstns commented 3 months ago

e2e tests should now be fixed. Please do not merge until parent https://github.com/FlowFuse/flowfuse/pull/4050 is approved/merged

@Steve-Mcl @joepavitt

joepavitt commented 3 months ago

@cstns https://github.com/FlowFuse/flowfuse/pull/4050 is not merging into main, it's merging into another branch. Which PR is #4050 dependant upon?

cstns commented 3 months ago

4050 is supposed to be merged in #3994 which is approved but not merged. I'll merge #3994 after the tests pass

cstns commented 3 months ago

done! this PR points to smtp_affair which then points to main

joepavitt commented 3 months ago

How do I test this? What URL do I need to navigate to in order to test this?

cstns commented 3 months ago

I mentioned the url format in https://github.com/FlowFuse/flowfuse/issues/3946#issuecomment-2149606548. Since I didn't receive any feedback I left it as <FlowFuse.domain>/deploy/blueprint?blueprintId=<xxx>.

In order to test this feature you'd need to be logged out of the FF app, access the url, follow registration form, login and get redirected to the instance creation flow with the correct blueprint selected.

Keep in mind that the FF platform must allow new users to register on the login screen and the creation of personal teams when users register enabled, otherwise you'd either be unable to register or be redirected to a page where you are prompted to get in touch with an admin.