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

Add the ability to control component group visibility by user authentication #1897

Closed
dbarentine opened this issue Jun 6, 2016 · 19 comments
Closed
Assignees
Milestone

Comments

@dbarentine
Copy link

dbarentine commented Jun 6, 2016

We also have certain component groups that we don't want visible to reduce end user confusion but we want our internal team members (who would have user accounts) to be able to see it on the status page. So it would be nice if we could control visibility based on three options, Viewable by public, Only visible to logged in users, and Hidden. Basically something similar to how Incident Visibility works.

Similar to issue 1892 but with finer grain level of visibility.

@jbrooksuk jbrooksuk modified the milestone: V3.0.0 Jun 7, 2016
@jbrooksuk jbrooksuk changed the title Add the ability to control component group visibility by user authentication. Add the ability to control component group visibility by user authentication Jul 12, 2016
@jbrooksuk
Copy link
Member

This could be part of #236, so I'll move this into the v2.5.0 milestone.

@yoyosan
Copy link
Contributor

yoyosan commented Jul 25, 2016

Hi.

I'd like to work on this since it's part of #1892 .

For the components group, I am thinking of:

  • adding a visible int column to the component_groups table with the three possible values mentioned above 0 - Viewable by public, 1 - Only visible to logged in users, 2 - Hidden. Default value is 2 - Hidden.
  • rephrase the existing Choose visibility of the group label to Expand/Collapse options.

What do you think?

@jbrooksuk
Copy link
Member

I like it! Looking forward to seeing your contribution @yoyosan!

@joecohens @GrahamCampbell and I are at Laracon US this week, so it might be that reviewing of your PR is delayed until we're back, but don't worry!

@yoyosan
Copy link
Contributor

yoyosan commented Jul 31, 2016

@dbarentine I'm not sure why you need the Hidden option.

Did you maybe thought the component group should be visible only to the creator?

@jbrooksuk do you use IRC or slack? I want to ask a couple of questions.

@yoyosan
Copy link
Contributor

yoyosan commented Jul 31, 2016

I can't figure out why running the tests is stuck for more than 5 minutes now.

Here's the output:

➜  Cachet git:(add-visibility-to-component-groups) ✗ phpunit -v tests
PHPUnit 4.8.21 by Sebastian Bergmann and contributors.

Runtime:        PHP 7.0.9
Configuration:  /home/yoyosan/Work/PHP/Cachet/phpunit.xml.dist

...............................................................  63 / 808 (  7%)
............................................................... 126 / 808 ( 15%)
............................................................... 189 / 808 ( 23%)
.............F...............................................F. 252 / 808 ( 31%)
............................................................... 315 / 808 ( 38%)
.............................................

@yoyosan
Copy link
Contributor

yoyosan commented Jul 31, 2016

The next step is to add functional tests for the code I've added for both the API and dashboard :)

@dbarentine
Copy link
Author

@yoyosan For our use we don't actually need the hidden option but I think I was trying to include it for completeness.

Specifically, I was trying to include the request from 1892 which wanted to hide the component group but still collect data on it via the API.

It could also be useful as you are creating new component group but don't want to expose it to anyone yet. So in that case exposing it only to the creator would be fine.

@yoyosan
Copy link
Contributor

yoyosan commented Aug 1, 2016

@dbarentine will add this too then.

@yoyosan
Copy link
Contributor

yoyosan commented Aug 1, 2016

Regarding phpunit, found the "fix" in #1721.

@yoyosan
Copy link
Contributor

yoyosan commented Aug 2, 2016

Things left to do:

  • Fix existing tests
  • Add dashboard functional tests for the displayed component groups as a guest vs logged in
    • Trying to skip the setup step while running the tests(harder than I thought) - found a temporary fix by copying boostrap/cachet/production.php to boostrap/cachet/testing.php
    • Test as guest
    • Take some time to understand how caching works in Laravel and implement a better way(automatically preferably) to skip the setup page while running tests.
    • Test as a signed in user
  • Add API tests for guest vs with Token requests
  • Using TDD, add hidden functionality for component groups Add the ability to enable/hide a component group #1892
    • UI
      • Add test/s for the index page
      • Add migration for a new column created_by in order to be able to determine the creator of a component group
      • Add new methods to the ComponentGroup and User models for the new column: ComponentGroup->createdBy() and User->componentGroups()
      • Add checks in place in order to make the test/s pass
      • Add test for the dashboard page
      • Save owner on create/edit component group
      • Update failing tests related to the Component group model
    • API
  • Code review
    • Add a better description to the PR
    • Rebase with upstream 2.4 and squash the commits
    • Get rid of the hidden visibility feature
  • Refactor time
    • Talk with @jbrooksuk about moving methods out of the StatusPageControllerTest class into the AbstractTestCase class.
    • Maybe move functional tests into their own directory e.g. tests/functional, integration tests into tests/integration, unit tests into tests/unit.
  • Update documentation for Component group on https://docs.cachethq.io/

@yoyosan
Copy link
Contributor

yoyosan commented Aug 6, 2016

Weird behaviour I've noticed:

  • On the index page(StatusPageController::showIndex), you need the component group to have at least one component in order to be displayed. For the API, the component group doesn't need to have any - Expected behaviour.

@jbrooksuk
Copy link
Member

@yoyosan where exactly do you mean in the dashboard? On the index?

@yoyosan
Copy link
Contributor

yoyosan commented Aug 6, 2016

Yes, on the index page(StatusPageController::showIndex). I've observed this while adding the API tests for component groups visibility. Didn't investigate yet though.

@jbrooksuk
Copy link
Member

Oh, not the dashboard, sorry I misunderstood.

@jbrooksuk
Copy link
Member

This is the expected behaviour, though. The UI shouldn't show an empty group, but the consumers of the API want to know about all groups.

@yoyosan
Copy link
Contributor

yoyosan commented Aug 6, 2016

Bad phrasing, sorry.

@jbrooksuk
Copy link
Member

Bad phrasing, sorry.

Nah, I'm just a numpty.

@yoyosan
Copy link
Contributor

yoyosan commented Aug 6, 2016

Nah, I'm just a numpty.

! true :)

@jbrooksuk jbrooksuk added feature and removed feature labels Jan 4, 2017
@jbrooksuk jbrooksuk modified the milestones: V2.5.0, V2.4.0 Jun 17, 2018
@jbrooksuk jbrooksuk modified the milestones: v2.4, v2.x Aug 12, 2023
@jbrooksuk
Copy link
Member

Thank you for your input on Cachet 2.x. We are shifting our attention and resources to Cachet 3.x and will no longer be supporting the 2.x version. If your feedback or issue is relevant to the 3.x series, we encourage you to engage with the new branch.

For more information on the Cachet rebuild and our plans for 3.x, you can read the announcement here.

We appreciate your understanding and look forward to your contributions to the new version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants