-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Use current package_type for urls #5189
Conversation
I've updated everything single place I was managed to locate by now. As for tests - I'm not sure if just visiting all possible pages and checking all the URLs related to dataset/resource views would do the thing... any idea on this account? |
Hi, I updated my base.html accordingly, however, I receive this error: is this due to python version? or what? Best regards |
Hi, @MandanaMoshref . Thus, this PR neither can be cherry-picked nor backported into v2.8 and the only thing you can do - manually re-define a bunch of templates and Generally, before v2.9, any trivial changes to metadata schema were assuming that you are still working with Dataset - i.e. it should be safe to use all the same routes and templates(unless you are using IDatasetForm directly, not via ckanext-scheming). If you are going to create some entity, that behaves in a completely different way(ckanext-scheming) - you should define a custom controller with custom routes in addition to changes to metadata schema. This means, for such purpose you shouldn't rely only on ckanext-scheming. |
Does this change let plugins override the views for specific dataset types? |
9e77b40
to
5bfc88a
Compare
Previously it didn't, but when you mentioned it, I've realized that change required for supporting extendible blueprints is quite trivial, so I've included it in the last commit. Here's the link, in case I'll add some style fixes before you'll have time to review changes: c832174 |
@smotornyuk this is great work. Good to merge? |
It will be really great if this PR will be merged and available in v2.9(we have a few projects that are currently using custom CKAN fork because this change is essential for their workflow). So, if you have no objections and or any ideas as to some additional improvements that can be useful in conjunction with ckanext-scheming - let's merge it |
Working with custom package types(especially, while using ckanext-scheming), I've noticed this quite a big problem in our templates/redirects. No matter, what kind of dataset you are creating, even though you are likely using URL like
/CUSTOM-PACKAGE-TYPE/new
, on progressing to the second step you'll be redirected to resource creation page on/DATASET/name/resource/new
.We are just usingdataset.edit
,resource.new
, etc. routes for URL generation, thuspackage_type
is ignored.Initially, i thought about hacking into
url_for
- this just requires few lines of code, but in this case, additional query into DB required and usage ofpackage_type
becomes implicit. Instead, even if it requires tons of changes, I'm leaning to direct updates of allurl_for
usages. Yep, it requires more work, but it:dataset.new
leads to/dataset/new
- you are still right