feat: Add a link to each service's analytics page in the editor #2387
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Mike-Heneghan
force-pushed
the
mh/analytics-link-to-analytics-in-editor
branch
from
0479b44 to
cba3f5f
Compare
|
🤖 Hasura Change Summary compared a subset of table metadata including permissions: Updated Tables (1)
|
Mike-Heneghan
changed the title
|
Removed vultr server and associated DNS entries |
DafyddLlyr
requested changes
Nov 6, 2023
DafyddLlyr
reviewed
Nov 6, 2023
...a.planx.uk/migrations/1699010412430_alter_table_public_flows_add_column_metabase_link/up.sql
Outdated
Show resolved
Hide resolved
Mike-Heneghan
added a commit
that referenced
this pull request
Nov 7, 2023
- As per: #2387 (comment) - Update the flowMetabaseLink to handle the query returning null
Mike-Heneghan
added a commit
that referenced
this pull request
Nov 7, 2023
…navailable - As per: #2387 (comment) - Move the analytics link to be the middle option to maintain groupings - As per: #2387 (comment) - Render a disable version of the analytics page link to covney active/inactive to improve usability
Mike-Heneghan
added a commit
that referenced
this pull request
Nov 7, 2023
- As per: #2387 (comment) - Best practice not to tie the variable name to a single provider
jessicamcinchak
approved these changes
Nov 8, 2023
There was a problem hiding this comment.
Looks good and all working as expected for me! I successfully added a (fake) analytics_link to this flow.
One minor comment about migration history, but not a particularly big deal to skip if you're ready to get this merged.
...a.planx.uk/migrations/1699010412430_alter_table_public_flows_add_column_metabase_link/up.sql
Outdated
Show resolved
Hide resolved
- Add analytcis link via bar chart icon - Only renders if the flow has a metabase link associated in the db
- As per: #2387 (comment) - Update the flowMetabaseLink to handle the query returning null
… and flowMetabaseLink - Consistently use the pattern of directly settings state attributes
…navailable - As per: #2387 (comment) - Move the analytics link to be the middle option to maintain groupings - As per: #2387 (comment) - Render a disable version of the analytics page link to covney active/inactive to improve usability
- As per: #2387 (comment) - Best practice not to tie the variable name to a single provider
- As the name of the new column changed there was a create and a rename migration - Removed both of these and added a new migration which names the column correctly from the get go
Mike-Heneghan
force-pushed
the
mh/analytics-link-to-analytics-in-editor
branch
from
b4e3f1d to
4ed6592
Compare
All seems good on this Pizza testing with this flow: https://2387.planx.pizza/testing/mikes-analytics-testing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
analytics_linkto theflowtable to store the link to analytics for each flowsharedstore.analytics_linkto open the page in a new tabWhy
Screen Recording
Screen.Recording.2023-11-06.at.17.28.29.mov
Follow Up
analytics_linkfor services which have them as per https://www.notion.so/opensystemslab/Plan-Service-Content-d579ddc3f4f3472b8fe167865ede2e93#e637abed931d4697a2707455d81e4620