Don't render tile for anon if unpublished nitf.

[idgserpro]

This fixes #185

Showing a default Plone message of "Insufficient Privileges" when an Anonymous user renders a tile that have an unpublished nitf associated to the tile.

[idgserpro]

I think we have the following issues here:

1) Unpublished content shouldn't be shown to anonymous users. This is default in Plone. You have to remember that a private content in a tile can be added to a tile, but with a website with multiple administrators, a nitf that once was published and inserted into a tile can be made private again too; 2) Logged-in users should have a visual indication that an item isn't published, if possible, using the same pattern that Plone uses (see https://github.com/collective/collective.nitf/issues/185#issuecomment-274800949); 3) The link to a content is coming as "None" and this should be fixed.

With that in mind, let's continue the discussion.

the cause of the problem is in the basic tile of collective.cover

Indeed. But we're doing specific cases first (https://github.com/collective/collective.cover/issues/288#issuecomment-274602798) and we believe that after solving a bunch of specific cases, we can try to add a generic solution to collective.cover. This issue was first spotted by a client in collective.nitf and that's why we're proposing it here first.

there's no reason not to have a tile linking private content

We're not removing the tile linking private content: we're removing the visualization of unpublished content to anonymous users.

This is what a logged in user sees with two private nitfs:

And an anonymous:

we need to fix the basic tile to correctly add the link to the private content, and that will probably fix also the problem here.

Fixing the link will not fix the problem here. The link comes as None in two situations: if the original content was deleted or if the user doesn't have the permission to see the item. Fixing the link won't fix the issue here that is showing private information to anonymous users.

This PR only solves 1), but the structure of this fix here can be used to solve the other problems.

For example: you could test if the user is logged in, if it has the correct permissions for messing up with the cover and if the link is None, is probably because the item ws deleted (it can also be moved to a folder where the user doesn't have permission, but you can use unrestrictedSearchResults for that). You can show a message ("Associated content was deleted") or even removing the uuid from the tile data manager when this happens: but these are decisions out of the scope of this PR.

[hvelarde]

we have discussed here and we understand your point; nevertheless the solution should be leave the tile empty and not show those ugly messages of "insufficient privileges"; that's the default in Plone listings.

so, resuming:

• authenticated users with privileges to see the content, should see the content in red (using the state-private class) like in the standard listings
• anonymous users and users without privileges should see no content at all

[idgserpro]

We agreed about removing the provided message, but when we do a return '', another message Tile content replaced during the blocks transform. is shown.

So, do we change Insufficient Privileges to Tile content replaced during the blocks transform.? At least is the default when a tile has a problem.

[hvelarde]

an alternative solution should be to create a function can_view() and use it in the template to display content or not.

you don't have to display an empty string, but the same markup that is shown when the tile is empty.

[idgserpro]

We didn't agree with adding a function to the view because we don't need this logic since the rendering machinery is already in place. Check the code for the documentation.

[hvelarde]

I'm still not sure on merging this as it would be better to fix the issue in collective.cover; I'll take a look earlier on monday.

[idgserpro]

We think that both can be done, but in different times: when collective.cover implements this (and with the visual indication that it's private) we can remove it from collective.nitf. Like you said, let's do the specific stuff first.

[hvelarde]

sorry, guys; I've been busy; I still have a question for you: are you using branch 2 of this package in your customer?

I'm asking this because I can merge this and later remove the modifications when we implement that upstream in collective.cover; also I'm about to make release of collective.cover and collective.nitf soon.

let me know.

[idgserpro]

We're not using it, but we plan to upgrade collective.nitf to 2.x after this fix is merged and a new release is made (and then close https://github.com/plonegovbr/brasil.gov.portal/issues/346).

I'm asking this because I can merge this and later remove the modifications when we implement that upstream in collective.cover

That's the idea, have at least a collective.nitf release with this fix since not all installations can have the latest collective.cover.

We have an idea for the private visual indication in collective.cover, we can provide more information when you add this upstream.

[idgserpro]

Just wait a minute, we need to change the commit message, it's not "Insufficient Privileges" anymore.

[hvelarde]

I have implemented an alternate solution upstream in collective.cover as there is a corner case you are not addressing.

[hvelarde]

@idgserpro I just tested this against the new collective.cover 1.5b1 and the issue is gone; I prefer you to create a patch for your customer instead of having to maintain redundant code here.