Sharing tab - Githubissues

frapell commented 3 years ago

I have made the sharing tab work mimicking how the Plone one works, with some limitations. This is mainly because on Plone you choose whether to inherit permissions from higher folders from the subitem itself, whereas in guillotina you decide when setting the permission, whether you want it to cascade below or not. Because of this, the "Inherit" checkbox doesn't work and it is always marked as true.

So, current implementation has the following caveats:

You cannot "Deny" a permission, you can only "Allow" or "Unset" it
If a permission is allowed in a folder, it will be inherited on subfolders
Inheriting from higher levels cannot be turned off.

frapell commented 3 years ago

@bloodbare Thoughts?

bloodbare commented 3 years ago

Do you know about https://github.com/plone/guillotina/blob/master/guillotina/security/security_local.py#L42 ?

bloodbare commented 3 years ago

I just released that GH Actions are failling on the previous PR, could you check it on this one ?

bloodbare commented 3 years ago

I think that nobody ever checked if they work ... would you fix it on this PR to avoid adding more code ?

frapell commented 3 years ago

@bloodbare Ahhh ! didn't know about that... I was following https://guillotina.readthedocs.io/en/latest/developer/security.html

I will include it!

as per the tests, I'll take a look

frapell commented 3 years ago

@bloodbare It appears there are some outdated tests that need to be adjusted, but most importantly guillotina_linkintegrity has errors, and for what I see in https://github.com/guillotinaweb/guillotina_linkintegrity , its last commit was from 2 years ago... before I start going down that rabbit hole, is that package still mantained? any advice?

bloodbare commented 3 years ago

That package was done by @vangheem and never updated. I don't know how much work would it mean, Nathan do you have any idea ?

vangheem commented 3 years ago

guillotina_linkintegrity is not maintained -- you can take over it if you like!

My brain is has far moved on from the package/cms/volto so I can't really give you any insight either. (sorry)

frapell commented 3 years ago

@bloodbare Alright, there are 2 remaining tests failing

test_editors.py There seems to be missing a guillotina_volto.interfaces.editors.IReactPageLayout, was this removed from the package at some point?
test_ob_position.py I am going to need some help with it, not sure how to properly fix this.

Quick question, do you have tips on debugging async code? specifically, is there a way to call awaitable functions from pdb? I have tried different things but cannot find a proper solution

bloodbare commented 3 years ago

You can remove IReactPageLayout, was a layout using react-page project instead of volto.

About test_ob_position which is the error ?

Async pdb works if you set a pdb before and after. You can step in an async func but if you get to a I/O call you will loose the pointer.

frapell commented 3 years ago

@bloodbare As per the PDB, I was expecting to be able to manually call functions, instead of adding breakpoints everywhere... I guess it cannot be done?


______________________________________________________________________________ test_get_max_position_in_folder _______________________________________________________________________________

cms_requester = <guillotina_volto.tests.fixtures.CMSRequester object at 0x7f08f47abdc0>

    @pytest.mark.skipif(
        os.environ.get("DATABASE", "DUMMY") in ("cockroachdb", "DUMMY"),
        reason="Not for dummy db",
    )
    async def test_get_max_position_in_folder(cms_requester):
        async with cms_requester as requester:
            await requester(
                "POST",
                "/db/guillotina/",
                data=json.dumps(
                    {
                        "@type": "Item",
                        "title": "Item1",
                        "id": "item1",
                        "@behaviors": [ICMSBehavior.__identifier__],
                    }
                ),
            )
            await requester(
                "POST",
                "/db/guillotina/",
                data=json.dumps(
                    {
                        "@type": "Item",
                        "title": "Item2",
                        "id": "item2",
                        "@behaviors": [ICMSBehavior.__identifier__],
                    }
                ),
            )

            root = await utils.get_root(db=requester.db)
            container = await root.async_get("guillotina")
            pos = await get_last_child_position(container)
>           assert pos > 1
E           assert 0 > 1

guillotina_volto/tests/test_ob_position.py:47: AssertionError
------------------------------------------------------------------------------------- Captured log setup -------------------------------------------------------------------------------------```

bloodbare commented 2 years ago

Sorry for the delay, crazy days here!

If you apply this patch will work again (did not commited because I don't have permissions on enfold fork)

index eaccb52..7ac13d6 100644
--- a/guillotina_volto/ordering.py
+++ b/guillotina_volto/ordering.py
@@ -26,7 +26,7 @@ async def get_last_child_position(folder):
     conn = await txn.get_connection()
     results = await conn.fetch(
         """select json from {}
-WHERE parent_id = $1 AND of IS NULL
+WHERE parent_id = $1 AND of IS NULL AND (json->>'position_in_parent')::int IS NOT NULL
 ORDER BY (json->>'position_in_parent')::int DESC
 limit 1""".format(
             txn.storage._objects_table_name

frapell commented 2 years ago

@bloodbare Thank you! tests are passing in my local env, can you run the actions on a pull request?

bloodbare commented 2 years ago

Can you add the pull actions on the github workflow definition so actions are run ?