Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Creation of datasets/harvest sources with no organization specified #3046

Closed
nibecker opened this issue May 23, 2016 · 7 comments

Comments

Projects
None yet
4 participants
@nibecker
Copy link

commented May 23, 2016

CKAN Version if known (or site URL)

2.5.2

Please describe the expected behaviour

Alternative 1: When adding a new dataset/harvest source and no organization has been created so far, the user should be promoted with a note that this has to be done first. Maybe the form could be grayed out.

Alternative 2: When adding a new dataset/harvest source and no organization has been created so far, the form should still include the organization-field. When the user puts focus on it, a dialogue opens assisting the user in creating a new organization.

Please describe the actual behaviour

When adding a new dataset/harvest source and no organization has been created so far, the form field for organization is not displayed. The form can be filled out. After saving an error message reads

The form contains invalid entries:
Owner org: A organization must be supplied

This is confusing since no such field is provided.

What steps can be taken to reproduce the issue?

  1. Create a new ckan instance or delete all organizations. 2. Create a new dataset/set up a new harvest source.
@nibecker

This comment has been minimized.

Copy link
Author

commented May 24, 2016

btw. "Owner org: An organization must be supplied" And maybe provided is better here …

k-nut added a commit to k-nut/ckan that referenced this issue May 24, 2016

@k-nut

This comment has been minimized.

Copy link
Contributor

commented May 24, 2016

With the change that I pushed to my branch the user is now notified that they need to create an organization first.

bildschirmfoto 2016-05-24 um 17 07 37

Again I am not sure if that is best way to do it. Maybe @wardi or @amercader have some input?

@TkTech

This comment has been minimized.

Copy link
Member

commented May 24, 2016

This seems like a reasonable and simple solution? Ideally for user workflow, after creating the organization you would be redirect back, but that's just fluff.

@k-nut

This comment has been minimized.

Copy link
Contributor

commented May 25, 2016

@k-nut

This comment has been minimized.

Copy link
Contributor

commented May 25, 2016

@TkTech I agree that it would be nice to redirect the user after they created the organization but I guess that it would not be worth the extra logic that we would need to introduce for this. This is a scenario that should not really happen too often so maybe we can just let the users navigate back by themselves.

k-nut added a commit to k-nut/ckan that referenced this issue May 25, 2016

@amercader

This comment has been minimized.

Copy link
Member

commented May 25, 2016

The output page looks fine, I'd probably make it look a bit more warning-y.

But the checks need to cover more cases.

You are using if not h.organizations_available('create_dataset') to decide whether to show the message or not but you also need to check that you actually require datasets to belong to an organization. IIRC this can be done adding and not h.check_config_permission('ckan.auth.create_unowned_dataset').

Then the problem that might happen is that datasets have to belong to an org, you are not an admin/editor member of any org (which is what you are checking with h.organizations_available('create_dataset')) but you can't actually create an organization (becasue of ckan.auth.user_create_organizations = false or custom auth). I'm pretty sure that if that happens you will get a Forbidden error before showing the page that you are updating now, but if not you can check if that is the case with h.check_access('organization_create') and if that fails you can show a different message that does not include the button.

k-nut added a commit to k-nut/ckan that referenced this issue May 25, 2016

@k-nut

This comment has been minimized.

Copy link
Contributor

commented May 25, 2016

@amercader I already thought that there would be some more cases to take care of ;)

I just pushed another change that should take care of this.

I also realized that my previous changes would always display the warning message and never the form because it is not possible in jinja to create a {% block %} conditionally. I am using an {% include %} now instead which seems to work better. I will continue by adding some tests now.

k-nut added a commit to k-nut/ckan that referenced this issue May 25, 2016

k-nut added a commit to k-nut/ckan that referenced this issue Jun 6, 2016

k-nut added a commit to k-nut/ckan that referenced this issue Jun 6, 2016

k-nut added a commit to k-nut/ckan that referenced this issue Jun 6, 2016

k-nut added a commit to k-nut/ckan that referenced this issue Jun 6, 2016

k-nut pushed a commit to k-nut/ckan that referenced this issue Jun 7, 2016

k-nut pushed a commit to k-nut/ckan that referenced this issue Jun 7, 2016

k-nut added a commit to k-nut/ckan that referenced this issue Jul 4, 2016

k-nut added a commit to k-nut/ckan that referenced this issue Jul 4, 2016

k-nut added a commit to k-nut/ckan that referenced this issue Jul 4, 2016

k-nut added a commit to k-nut/ckan that referenced this issue Jul 4, 2016

k-nut pushed a commit to k-nut/ckan that referenced this issue Jul 4, 2016

k-nut pushed a commit to k-nut/ckan that referenced this issue Jul 4, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.