Private datasets created from the start as such appear in the user's public activity stream #953
Conversation
ghost
assigned 
|
@nigelbabu Don't steal this one, I've started working on it.. |
Add tests that no activities are emitted when creating, updating or deleting private datasets via the API. These could be more detailed, e.g. test adding, deleting and updating groups as well, or moving the dataset between groups or between orgs, or testing what happens when a private dataset is made public or vice-versa.
Move the visibility option (public or private) when creating a dataset to the first stage of the dataset creation form. This fixes the issue (#953) that an activity is emitted when creating private datasets. Since the dataset is actually created when the first form is submitted, and the public/private setting is not chosen until the third form, the dataset is public at the moment when it's created and an activity is emitted. It seems to make a lot more sense fo the visibility option to be on the first form next to the organization option anyway. Fixes #953.
If a dataset has no owner organization, authorize anyone to read that dataset (regardless of whether it's public or private).
Fix the logic for displaying the Organization and Visibility dropdowns when creating or updating a dataset. We really need frontend tests for this. I tested these cases manually: User is not logged in and anon_create_dataset = True: neither Organization nor Visibility shows, either when creating or when updating a dataset. User is logged in, but is not a member of any organization: neither Organization nor Visibility shows, either when creating or when updating a dataset. Tested both when the site has no organizations and when the site does have orgs but user is not a member of any. User is logged in, and is a member of an organization: Both Organization and Visibility show when creating a dataset. When creating a private dataset, no activities appear in the activity stream. User is logged in, and is a member of an organization, but dataset is not a member of any org: neither Organization nor Visibility shows when updating a dataset. User is logged in, and is a member of an organization, and dataset is a member of any org: Visibility shows but Organization does not show when updating a dataset. User is sysadmin, site has no orgs: neither Organization nor Visibility shows when creating or updating a dataset. User is sysadmin, site does have orgs: both Organization and Visibility show when creating or updating a dataset.
|
Alright. The problem here is caused by the three-stage dataset creation form. The Visibility (Public/Private) option is on the third form, but the dataset is actually created when you submit the first form. At the time when the dataset is created it is public, so a "seanh created the dataset foobar" activity is emitted. I think this activity might be visible on pages such as the dataset's organization's page, or the profile page of the user who created it, so there is private information leaking out here. We really need frontend tests. I'm not sure if we should merge this branch as it is, or if more work should be done it such as adding tests or changing the logic to force datasets with no organization to be public (as suggested below). Here's what I've done so far:
The logic for showing these dropdowns is in templates. It should really probably be moved into auth functions, maybe new This is another thing we really need frontend tests for as there are many possible cases. I tested the following cases manually, it'd be good if whoever reviews this could also do their own tests:
|
Don't allow non-sysadmins to read inactive datasets. This fixes a test that was failing.
|
As discussed with @wardi on IRC, we think we should add a validation (and test) that you cannot create a private dataset with no owner organization. Some javascript to prevent private and no owner org from being selected at the same time would be a bonus. |
Add a validator to prevent private datasets with no organization from being created by package_create or package_update.
|
Ok, I've added a validator so that private datasets with no organization cannot be created by @johnmartin What do you think about using javascript to force the Visibility dropdown to be Public when no organization is selected in the Organization dropdown? Is there an example of doing something like this somewhere in CKAN, that I can copy? Unfortunately the error message that's returned is:
because the key that is validated is called |
|
@seanh Yes, I'm fine with hiding it with JS as it doesn't prevent the form from working without JS it just improves the experience. In terms of an example I can think of one at the moment... but I'm happy to jump on this branch and do the JS needed tomorrow. Essentially the behaviour is: When no org is selected make visibility is public and user cannot change it. |
|
@johnmartin I go if you can do the JS on this branch, then I'll take a look at how you did it. I want to learn more about how we do the JS and other frontend stuff but I'm basically clueless about it right now and there never seems to be time to take out to study it I suggest the behaviour is:
Basically submitting the form with no organization and visibility private is invalid, so we don't want it to be possible to submit that |
Essentially we get the current value of the visibility dropdown and then if no org is selected then we disable the visibilty dropdown and set it to be public.
|
Looks good to me, I think someone can review this now |
|
@seanh a couple of the tests are broken(e.g test_03_create_update_package, see the travis build), due to the validator requiring private datasets to have an organisation, looks like most of them can be fixed quite easily with just creating a test organisation for those tests. |
This test was trying to create private datasets that don't belong to any organization, which isn't allowed.
|
@joetsoi It's good now I think |
|
P.S. Looks like this is going to get merged after the release-v2.1 branch is created, so could you get it merged into release-v2.1 as well? (Email Adria and ask him to cherry-pick it) |
|
@joetsoi I've re-run these tests on Travis and locally and they passed again (the one fail in the Travis build is a false positive) |
…n-activity Conflicts: ckan/logic/schema.py ckan/logic/validators.py ckan/public/base/javascript/resource.config
|
@seanh, I was being incredibly dense. The build was failing for me because of new datastore tests in master that have been added since this PR was opened. I've merged master into this branch An organization needs to be created in the setup_class of two test classes |
create an organization in setup and use it in package_create
Add tests that no activities are emitted when creating, updating or deleting private datasets via the API. These could be more detailed, e.g. test adding, deleting and updating groups as well, or moving the dataset between groups or between orgs, or testing what happens when a private dataset is made public or vice-versa.
Move the visibility option (public or private) when creating a dataset to the first stage of the dataset creation form. This fixes the issue (#953) that an activity is emitted when creating private datasets. Since the dataset is actually created when the first form is submitted, and the public/private setting is not chosen until the third form, the dataset is public at the moment when it's created and an activity is emitted. It seems to make a lot more sense fo the visibility option to be on the first form next to the organization option anyway. Fixes #953.
If a dataset has no owner organization, authorize anyone to read that dataset (regardless of whether it's public or private).
Fix the logic for displaying the Organization and Visibility dropdowns when creating or updating a dataset. We really need frontend tests for this. I tested these cases manually: User is not logged in and anon_create_dataset = True: neither Organization nor Visibility shows, either when creating or when updating a dataset. User is logged in, but is not a member of any organization: neither Organization nor Visibility shows, either when creating or when updating a dataset. Tested both when the site has no organizations and when the site does have orgs but user is not a member of any. User is logged in, and is a member of an organization: Both Organization and Visibility show when creating a dataset. When creating a private dataset, no activities appear in the activity stream. User is logged in, and is a member of an organization, but dataset is not a member of any org: neither Organization nor Visibility shows when updating a dataset. User is logged in, and is a member of an organization, and dataset is a member of any org: Visibility shows but Organization does not show when updating a dataset. User is sysadmin, site has no orgs: neither Organization nor Visibility shows when creating or updating a dataset. User is sysadmin, site does have orgs: both Organization and Visibility show when creating or updating a dataset.
Don't allow non-sysadmins to read inactive datasets. This fixes a test that was failing.
Add a validator to prevent private datasets with no organization from being created by package_create or package_update. Conflicts: ckan/logic/schema.py ckan/logic/validators.py
Essentially we get the current value of the visibility dropdown and then if no org is selected then we disable the visibilty dropdown and set it to be public. Conflicts: ckan/public/base/javascript/resource.config
This test was trying to create private datasets that don't belong to any organization, which isn't allowed.
create an organization in setup and use it in package_create
|
Looks like this got merged, thanks @joetsoi. I guess you emailed Adria for merge into 2.1? |
|
@seanh , I told adria it was all fine but the someone needed to fix the tests while you were away and we agreed I'd fix the test and he'd merge it in. So hopefully he remembered to merge it into 2.1 as well! |
smotornyuk
deleted the
953-bug-creating-a-private-dataset-emits-an-activity
branch
Successive edits don't.