-
Notifications
You must be signed in to change notification settings - Fork 2k
Conversation
LGTM. I've tested this and it works as expected. @codydaig As a guest (not logged in)... I did notice that if there aren't any Articles in the list, then the No articles yet... is displayed with a link to |
doesn't it also require update on the server side to allow articles to be retrieved? |
@lirantal The ACL for the Articles api was already configured to allow Guests to retrieve Articles. This PR is merely matching the front-end behavior to reflect this. |
Got it. That PR is already merged? :) I may have lost track |
@lirantal Yes, I believe the back-end ACL's have been this way for some time. Either way, it's definitely in master now. |
@codydaig @mleanos @lirantal does anybody else feel like the view that we have now is really meant for someone managing the articles, not intended for displaying to users? The API is configured correctly, and the UI does prevent access to the list of articles, but I feel like this is intentional? Are we "ok" with exposing the same "admin" type of interface (list) to users? Perhaps this is a good time to create the articles admin (anyone able to manage articles)? |
@rhutchison +1 on implementing the Admin feature for Articles. I can see the need for a separate view for Guests. Would definitely make it more secure and could provide more focus for both views; allowing for more robust feature sets in each. |
Just to make it clear that the admin module is there as an example module for the framework, so whatever we choose to enhance there on the articles area (which IMO is blessed) should be contained to that area. |
@rhutchison @mleanos Is there anything that I need to change in this PR? |
@codydaig I think not. Since this is technically a bug in the front-end routing config. After this is merged, we will need to discuss the Admin feature for Articles & the separation of the articles list views for Guests, and authenticated users. Each should be it's own PR. |
Ok great, I'll merge. |
Allow Guests to view Articles
Currently, the API on the server allows anybody (regardless if signed in or not) to retrieve a list of articles. However, the client requires the user to be logged in to view the articles. This makes it so visitors that are not authenticated to view the articles.
This goes along with #780 that makes the TopBar visible by default.