ckan / ckanext-hierarchy

Organization hierarchy - CKAN extension
GNU Affero General Public License v3.0
28 stars 64 forks source link

Clarify show function to use #63

Closed avdata99 closed 1 year ago

avdata99 commented 1 year ago

Probably user will require to continue using organization show function for new custom types. This PR:

amercader commented 1 year ago

@smotornyuk @avdata99 is there a situation where users would like to call anything other than group_show or organization_show? I think it's very unlikely that users would have registered something like my_custom_org_type_show and even so, these functions probably just need the standard fields returned by group_show or organization_show. In this case I think the original logic just needs to be modified to something like:


def group_tree_parents(id_, type_='organization'):
    model_obj = model.Group.get(id_)
    action_name = "organization_show" if model_obj.is_organization else "group_show"
    tree_node = p.toolkit.get_action(action_name)({}, {'id': id_,
                                                         'include_dataset_count': False,
                                                         'include_users': False,
                                                         'include_followers': False,
                                                         'include_tags': False})

WDYT?

smotornyuk commented 1 year ago

Has sense. I'd even say, that creating custom xxx_show functions for existing entities in an anti-pattern, given the fact that we can rely on type and custom schemas.

And this type_ is not used further so I'd completely remove it, if we have these two lines in the beginning of group_tree_parents

avdata99 commented 1 year ago

This was merged. Should I create a new PR to apply @amercader changes and drop type_? Or you have different plans @smotornyuk ?

amercader commented 1 year ago

Yes @avdata99 , a fresh PR would be the best