humhub / wiki

Create and edit pages with this powerful tool. Build a collaborative knowledge base, view and share information with your colleagues.
28 stars 35 forks source link

Public pages doesn't appear on the overview #334

Open marc-farre opened 3 months ago

marc-farre commented 3 months ago

E.g., if I am not a member of the Hosting space, I can see the wiki pages in the stream, but not in the wiki overview:

image

image

yurabakhtin commented 3 months ago

@marc-farre I see the parent page Homepage is not public, so all child pages of the page cannot be listed under the hidden parent page. If you open the child page "Jitsi Meet servers" you can view it.

@luke- Do you think we should do something for such case?

luke- commented 3 months ago

Hmm, is the page displayed under "Last Edited"?

Looks a bit broken somehow, I'm afraid it's difficult to find a solution here.

marc-farre commented 3 months ago

@yurabakhtin thanks, that's true. And the "List categories" page works well: https://community.humhub.com/s/hosting/wiki/overview/list-categories

However, shouldn't we throw a “Not allowed” error when viewing a private page without being a member of the space, instead of a blank page?

luke- commented 3 months ago

@marc-farre

However, shouldn't we throw a “Not allowed” error when viewing a private page without being a member of the space, instead of a blank page?

Unfortunately I can't find the issue right now, but I would like to have a generic handling for such cases.

("You need to become member", "Access Denied", "You need to login", ...)

It should not be necessary to implement this at module level.

Unfortunately, it is complicated and we have not yet found a nice solution.

Related: https://github.com/humhub/humhub-internal/issues/295

yurabakhtin commented 3 months ago

Hmm, is the page displayed under "Last Edited"?

last-edited

However, shouldn't we throw a “Not allowed” error when viewing a private page without being a member of the space, instead of a blank page?

The page tree is displayed by recursion, i.e. firstly we find all root pages and then find its children. If we will display in the sidebar page tree all even not allowed to view by current user pages, then it may looks like this:

I think it make sense to display a private parent page only if at least child is viewable for a current user, i.e. the first root parent page should be hidden with its children completely.

marc-farre commented 3 months ago

To make things clearer, we have 3 different pages (sorry, I was unclear with this):

"Homepage" is a content and can be private or public. If think that if it's private, we should have an "Access Denied".

"List categories" and "Last Edited" are not a content (no private/public status). So I agree with your behavior proposal, @yurabakhtin

Also, maybe we could prevent having a child page public if the parent is private.

luke- commented 3 months ago

Thanks for the explanation @marc-farre !

What is the proposal of @yurabakhtin? Not sure I fully got it.

There is also the scenario that no Homepage defined. Can't we just assume that if it is "Private", but the user can only see "Public" content?

yurabakhtin commented 3 months ago

"Homepage" is a content and can be private or public. If think that if it's private, we should have an "Access Denied".

@marc-farre Currently we display "Wiki page not found!" for all cases because we filter it by WikiPage::find()->readable(), but if need I think we can split and display "Access Denied" for case !$page->content->canView().

Also, maybe we could prevent having a child page public if the parent is private.

For this we need to update all children recursively to private when the parent is updated to private, I am not sure how it is normal if it was changed by mistake, because I don't think we should do the reverse action when the parent page is updated from privte to public. So at least we must display a warning about changing all children pages to private.


What is the proposal of @yurabakhtin? Not sure I fully got it.

@luke- My proposal was not completed because I am not sure what solution is better.

For example, we have such pages:

If current user has no access to the private pages then display them as:

1st solution:

2nd solution:

Currently we have the 2nd solution, it is why the issue ticket was creted here. We can keep it as is, but probably we need to display the "Access Denied" as @marc-farre suggested.

luke- commented 3 months ago

I would keep the second (currently existing solution) for now. It is really difficult to display a "tree" if just parts are visible to the user. This case should also be rather rare. I think Marc was primarily concerned with the "homepage". correct?

yurabakhtin commented 3 months ago

@luke- Ok, done in the commits https://github.com/humhub/humhub/pull/7094/commits/bd3b356b2f7e575fcd6fa75155a3aa1fd5d7e378, https://github.com/humhub/calendar/pull/492/commits/37a189cbab4f7b4d0448d6e6154dd7fd86ae8611.

marc-farre commented 3 months ago

I think Marc was primarily concerned with the "homepage". correct?

Yes.

@yurabakhtin thanks for the commits!

luke- commented 3 months ago

@yurabakhtin Your commits are related to the Calendar Module, I'll hide it here :-)

yurabakhtin commented 3 months ago

@luke- Oh yes sorry 🤦

yurabakhtin commented 3 months ago

@luke-

I think Marc was primarily concerned with the "homepage". correct?

Please confirm if we need to display a separate message "Access Denied" for case if user really has no access instead of current "Wiki page not found!" which is displayed for all cases.

luke- commented 3 months ago

Hmm, not sure right now. What is meant with "Access Denied"?

To sum up the problem: Public pages of Private Parent Pages are not listed in the overview?

We can either:

Or did I misunderstood something?

marc-farre commented 3 months ago

I think private should remain invisible, so current solution is fine.

After a better analysis of the issue (I should have done it before, sorry), I found out that clicking on the "Wiki" Space menu item displays a different page depending on the Space membership.

image

This issue is about the latter case:

image

image

What about the following idea?

On the "List categories" and "Last Edited", if no page links are displayed, show a message such as: "No public wiki pages":

image

Similar to an empty stream or grid.

yurabakhtin commented 3 months ago

Hmm, not sure right now. What is meant with "Access Denied"?

@luke- Here we have the code:

        $page = WikiPage::find()
            ->contentContainer($this->contentContainer)
            ->readable()
            ->andWhere(['wiki_page.id' => $id])
            ->one();

        if (!$page) {
            throw new HttpException(404, 'Wiki page not found!');
        }

If we will remove the filter ->readable() it may looks like this:

        $page = WikiPage::find()
            ->contentContainer($this->contentContainer)
            ->andWhere(['wiki_page.id' => $id])
            ->one();

        if (!$page) {
            throw new HttpException(404, 'Wiki page not found!');
        }

        if (!$page->content->canView()) {
            throw new HttpException(403, 'Access Denied!');
        }
luke- commented 3 months ago

Hmm, not sure right now. What is meant with "Access Denied"?

@luke- Here we have the code:

@yurabakhtin Thanks for the example. I wouldn't change that for now. We also have the problem in other places and I would like to find a core solution here.

luke- commented 3 months ago

Related: https://github.com/humhub/humhub/pull/6005