Skip to content

Hotfix/api umbrella analytics data#1752

Merged
brylie merged 9 commits intodevelopfrom
hotfix/api-umbrella-analytics-data
Oct 11, 2016
Merged

Hotfix/api umbrella analytics data#1752
brylie merged 9 commits intodevelopfrom
hotfix/api-umbrella-analytics-data

Conversation

@frenchbread
Copy link
Copy Markdown
Contributor

@frenchbread frenchbread commented Oct 11, 2016

Closes #1751

Changes

  • Added /api-umnrella/ prefix to ES query
  • Added UI option for API Umbrella Logs
  • Permission checks
  • Fix publication
  • i18n strings

@frenchbread
Copy link
Copy Markdown
Contributor Author

@apinf/developers please review

@brylie
Copy link
Copy Markdown
Contributor

brylie commented Oct 11, 2016

@frenchbread this works, but there are some usability issues:

  • we need to let the admin user know what they are seeing
    • to this end, we should provide an option for "Admin API" in the dropdown menu
    • If possible, the "Admin API" should include the proxy.name field, so the Admin user knows what they are viewing

Example screenshot

screenshot_20161011_115147

I made the above screenshot with the following code:

<optgroup label="Administrator" >
    <option value="/api-umbrella" >
      Proxy Admin API
    </option>
</optgroup>

@brylie
Copy link
Copy Markdown
Contributor

brylie commented Oct 11, 2016

Ping @bajiat @philippeluickx

@brylie
Copy link
Copy Markdown
Contributor

brylie commented Oct 11, 2016

@frenchbread basically, I think you can just wrap the above code example in an {{#if isInRole 'admin'}} block, and we can merge this PR.

{{#if isInRole 'admin'}}
<optgroup label="Administrator" >
    <option value="/api-umbrella" >
      Proxy Admin API
    </option>
</optgroup>
{{/ if }}

Note: Be sure to include i18n tokens.

@brylie
Copy link
Copy Markdown
Contributor

brylie commented Oct 11, 2016

@frenchbread after reviewing with @bajiat we think it would be good to remove the API Umbrella Logs option from the My APIs section,

screenshot_20161011_115844

The menu should contain an Administrator section with Proxy Admin API section:

screenshot_20161011_115147

@brylie
Copy link
Copy Markdown
Contributor

brylie commented Oct 11, 2016

@bajiat is this your understanding of our discussion? If so, this PR is really close to ready.

@frenchbread
Copy link
Copy Markdown
Contributor Author

So how much sections are we having in the end and why removing MyApis ?

@bajiat
Copy link
Copy Markdown
Contributor

bajiat commented Oct 11, 2016

@frenchbread We are not suggesting to remove MyAPIs section. @brylie just had not added any APIs when he took the screenshot.
Keep current sections. Just add an Administrator section and move the Proxy Admin API there. Also, rename "API Umbrella Logs" as "Proxy Admin API"

@brylie
Copy link
Copy Markdown
Contributor

brylie commented Oct 11, 2016

Right, for clarification:

  • remove API Umbrella Logs from the My APIs section
  • add an Administrator section
    • add Proxy Admin API to the Administrator section

screenshot_20161011_120940

@frenchbread
Copy link
Copy Markdown
Contributor Author

Done

@frenchbread
Copy link
Copy Markdown
Contributor Author

frenchbread commented Oct 11, 2016

@apinf/developers Please review

@brylie brylie modified the milestone: Sprint 33 Oct 21, 2016
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.

Allow Admin users to see Admin API analytics on the Dashboard

3 participants