Skip to content

feat: Filter networks by window location#100

Merged
stanleyjones merged 4 commits intoliftedinit:mainfrom
stanleyjones:connect-to-end
Dec 6, 2022
Merged

feat: Filter networks by window location#100
stanleyjones merged 4 commits intoliftedinit:mainfrom
stanleyjones:connect-to-end

Conversation

@stanleyjones
Copy link
Copy Markdown
Contributor

@stanleyjones stanleyjones commented Dec 6, 2022

Description

This PR adds a network for the END Labs ledger and filters which networks are available based on the location reported by the browser.

  • Add "GBOP Ledger" as a network option with the endpoint /api-end
  • Rename "Manifest Network" to "Manifest Ledger"
  • Filter available networks by browser-reported location

Closes https://github.com/liftedinit/operations/issues/6 (but doesn't really)

Testing / Breaking Changes

The change was tested manually and required modifying the existing E2E tests, as they were built with the assumption that only one network would be available.

BREAKING CHANGE: The "Manifest Network" will no longer be available from front-ends running Alberto that do not have "alberto" somewhere in the domain name. While this change is the goal of this ticket, it also constitutes a breaking change. We can put it back once ledger clusters have static URLs for their APIs.

Screenshots

AS ALBERTO

Screenshot 2022-12-05 at 2 28 05 PM

AS WALLET.END-LABS

Screenshot 2022-12-05 at 2 29 05 PM

@netlify
Copy link
Copy Markdown

netlify Bot commented Dec 6, 2022

Deploy Preview for lifted-alberto ready!

Name Link
🔨 Latest commit a1d863a
🔍 Latest deploy log https://app.netlify.com/sites/lifted-alberto/deploys/638f9107b4834b0008d23dac
😎 Deploy Preview https://deploy-preview-100--lifted-alberto.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Copy Markdown
Contributor Author

@stanleyjones stanleyjones left a comment

Choose a reason for hiding this comment

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

Some hopefully useful comments.

Comment thread config-overrides.js
Comment on lines +11 to +16
devServer: function (configDevServer) {
return function (proxy, allowedHosts) {
const config = configDevServer(proxy, allowedHosts)
return { ...config, public: "alberto.local" }
}
},
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This bit allows Webpack to serve Alberto locally at a specific domain (e.g. https://alberto.local:3000) provided you also add the following line to your /etc/hosts:

127.0.0.1  alberto.local
127.0.0.1  wallet.end-labs.local

It's useful in local development to test whether the hostname detection is working.

Comment on lines +41 to +45
const { activeNetwork, networks, setActiveId } = useNetworkStore(s => ({
networks: s.getNetworks(),
activeNetwork: s.getActiveNetwork(),
setActiveId: s.setActiveId,
}))
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

All of this business logic was moved out of the view and into the state store. See below.

Comment thread src/features/network/store.ts Outdated
Comment on lines +10 to +11
[0, { name: "Manifest Ledger", url: "/api", filter: "alberto" }],
[1, { name: "GBOP Ledger", url: "/api-end", filter: "end-labs" }],
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jgryffindor Please double-check the url here. I'm pretty sure it's what I heard you say. Easy to change.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good!

Comment thread src/features/network/store.ts Outdated
Comment on lines +31 to +35
.map(([id, network]) => ({ id, ...network }))
.sort(({ name: a }, { name: b }) =>
a.toLowerCase() < b.toLowerCase() ? -1 : 1,
)
.filter(({ filter }) => !filter || hostname.includes(filter))
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The mapping, sorting, and filtering has been moved here from the view.

Comment thread config-overrides.js
Comment on lines +14 to +15
return config
// return { ...config, public: "wallet.end-labs.local" }
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comment out L14 and uncomment L15 to serve the app from a different domain name during local development. Make sure to add that domain in your /etc/hosts as well (e.g. 127.0.0.1 wallet.end-labs.local).

Comment thread e2e/tests/home.spec.ts
Comment on lines -103 to +104
expect(networkName).toBe("no network selected")
expect(networkName).not.toBe(name)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The test assumed that no network would be available after deleting one. I changed it to confirm that the deleted network was no longer active.

Comment on lines +32 to +41
.map(([id, network]) => ({ id, ...network }))
.sort(({ name: a }, { name: b }) =>
a.toLowerCase() < b.toLowerCase() ? -1 : 1,
)
.filter(
({ filter }) =>
!filter ||
hostname.includes(filter) ||
hostname.includes("localhost"),
)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

All the mapping, sorting, and filtering from the view has been moved here. Note that all networks are available if you're just viewing from localhost — but see above for a trick to set your domain to something else in local development.

Comment on lines +45 to +47
const networks = get().getNetworks()
const activeNetwork = networks.find(({ id }) => id === get().activeId)
return activeNetwork ?? networks[0]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's possible that the "active network" in your browser cache no longer exists, in which case fall back to the first one alphabetically.

@stanleyjones stanleyjones self-assigned this Dec 6, 2022
@stanleyjones stanleyjones marked this pull request as ready for review December 6, 2022 00:42
@stanleyjones stanleyjones requested a review from rfisch December 6, 2022 00:43
@stanleyjones stanleyjones merged commit d650a7d into liftedinit:main Dec 6, 2022
@stanleyjones stanleyjones deleted the connect-to-end branch December 6, 2022 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants