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

Final touches for Quickstarts #2962

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions app/controllers/frontend_controller.rb
Expand Up @@ -37,6 +37,8 @@ class FrontendController < ApplicationController

layout :pick_buyer_or_provider_layout

helper_method :quickstarts_presenter

private

def disable_x_content_type
Expand Down Expand Up @@ -168,4 +170,17 @@ def render_wrong_domain_error
def set_display_currency
ThreeScale::MoneyHelper.display_currency = current_account.currency if current_account
end

def quickstarts_presenter
return if @quickstarts_presenter

current_api = if @backend_api&.id
@backend_api
elsif @service&.id
@service
else
current_account.default_service
end
@quickstarts_presenter = QuickstartsPresenter.new(current_api)
end
josemigallas marked this conversation as resolved.
Show resolved Hide resolved
end
16 changes: 15 additions & 1 deletion app/javascript/packs/quickstarts/container.js
@@ -1,6 +1,7 @@
// @flow

import { QuickStartContainerWrapper as QuickStartContainer } from 'QuickStarts/components/QuickStartContainer'
import { getActiveQuickstart } from 'QuickStarts/utils/progressTracker'
import { safeFromJsonString } from 'utilities/json-utils'

document.addEventListener('DOMContentLoaded', () => {
Expand All @@ -12,9 +13,22 @@ document.addEventListener('DOMContentLoaded', () => {
}

const { links, renderCatalog } = container.dataset
const parsedRenderCatalog = safeFromJsonString<boolean>(renderCatalog)
const willRenderQuickStartContainer = getActiveQuickstart() || parsedRenderCatalog

if (!willRenderQuickStartContainer) {
container.remove()
return
}

QuickStartContainer({
links: safeFromJsonString<Array<[string, string, string]>>(links) || [],
renderCatalog: safeFromJsonString<boolean>(renderCatalog)
renderCatalog: parsedRenderCatalog
}, containerId)

const wrapperContainer = document.getElementById('wrapper')
const quickStartsContainer = document.querySelector('.pfext-quick-start-drawer__body')

// $FlowIgnore HACK of the year: We need QuickStartContainer to wrap the whole #wrapper for the Drawer to work properly.
quickStartsContainer.after(wrapperContainer)
})
13 changes: 13 additions & 0 deletions app/javascript/src/QuickStarts/utils/progressTracker.js
@@ -0,0 +1,13 @@
// @flow

/**
* This is a collection of wrappers around localStorage to ease getting information about Quickstarts state
*/

function getActiveQuickstart (): null | string {
Copy link

@jkeam jkeam Apr 22, 2022

Choose a reason for hiding this comment

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

I think you can do something like:

function getActiveQuickstart(): null | string {
  const quickstartId = localStorage.getItem('quickstartId')
  return quickstartId || null
}

From MDN (https://developer.mozilla.org/en-US/docs/Web/API/Storage/getItem), it returns: A string containing the value of the key. If the key does not exist, null is returned.

I don't think there's a need to JSON.parse as it looks like it's an ID you are saving and then retrieving.

And no need to check for length as the getItem call will already return either the string or null if the key does not exist. The reason for the short circuit logic in the return is to catch the edge case of quickstartId being an empty string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's the catch: the Patternfly component stores this value as a stringyfied JSON, so the possible values are

undefined
"\"\""
"\"creating-a-method-quick-start\""

So we first need to check if it's a valid JSON and if so, if it's an empty string 😂

I forgot to add some tests, I hope it'll be clearer now

Copy link

@jkeam jkeam Apr 27, 2022

Choose a reason for hiding this comment

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

Ah I see. Then maybe we can make the variable named better? Anything named 'id' is usually a number (or sometimes a string for something like a guid); but in the case of an object, maybe a better name here? Just an idea.

// $FlowIgnore[incompatible-type] getItem returns string null
const data: null | string = localStorage.getItem('quickstartId')
return (data && data.length && JSON.parse(data)) || null
}

export { getActiveQuickstart }
6 changes: 0 additions & 6 deletions app/lib/logic/rolling_updates.rb
Expand Up @@ -296,12 +296,6 @@ def enabled?
end
end

class QuickStarts < Base
def missing_config
false
end
end

class Unknown < Base
def missing_config
false
Expand Down
1 change: 1 addition & 0 deletions app/presenters/quickstarts_presenter.rb
Expand Up @@ -11,6 +11,7 @@ def initialize(current_api)

attr_reader :current_api

# Consumed by app/javascript/src/QuickStarts/utils/replaceLinksExtension.js
def links
[
['create-a-product-link', new_admin_service_path, 'Create a product'],
Expand Down
11 changes: 1 addition & 10 deletions app/views/shared/provider/_quickstarts_container.html.slim
@@ -1,11 +1,2 @@
#quick-start-container data-render-catalog=(active_menu == :quickstarts).to_s data-links=QuickstartsPresenter.new(current_api).links
#quick-start-container data-render-catalog=(active_menu == :quickstarts).to_s data-links=quickstarts_presenter.links
= javascript_pack_tag 'quickstarts/container'

javascript:
document.addEventListener('DOMContentLoaded', () => {
// HACK of the year: We need QuickStartContainer to wrap the whole #wrapper for the Drawer to work properly.
const wrapperContainer = document.getElementById('wrapper')
const quickStartsContainer = document.querySelector('.pfext-quick-start-drawer__body')

quickStartsContainer.after(wrapperContainer)
})
2 changes: 1 addition & 1 deletion config/examples/features.yml
Expand Up @@ -17,7 +17,7 @@ features: &default
logging:
audits_to_stdout: true
quickstarts:
enabled: false
enabled: true
email_configuration:
enabled: false

Expand Down
1 change: 1 addition & 0 deletions features/provider/header.feature
Expand Up @@ -4,6 +4,7 @@ Feature: Header buttons and menus
Given a provider is logged in

Scenario: Help menu dropdown
Given quickstarts is disabled
Then the help menu should have the following items:
| Customer Portal |
| 3scale API Docs |
Expand Down
Expand Up @@ -65,7 +65,7 @@

find("[data-test='tile #{test_quickstart_id}']").click

assert_selector "[data-testid='qs-drawer-#{test_quickstart_id.underscore.camelize(:lower)}']"
assert_selector '[data-test="quickstart drawer"]'
assert_selector('[data-testid="qs-drawer-side-note-action"]', text: 'Restart')
end

Expand Down
14 changes: 14 additions & 0 deletions spec/javascripts/QuickStarts/utils/progressTracker.spec.js
@@ -0,0 +1,14 @@
// @flow

import { getActiveQuickstart } from 'QuickStarts/utils/progressTracker'

it('should return null when there is no active quickstart', () => {
localStorage.removeItem('quickstartId')
expect(getActiveQuickstart()).toBe(null)

localStorage['quickstartId'] = '""'
expect(getActiveQuickstart()).toBe(null)

localStorage['quickstartId'] = '"creating-a-method-quick-start"'
expect(getActiveQuickstart()).not.toBe(null)
})