organization_item.html shows package list instead of count

[seanh]

If an organization has more than 0 datasets then organization_item.html shows the list of packages (as an unformatted json list) instead of the count

[seanh]

@johnmartin Not sure if this is needed for 2.0. It seems odd that we didn't notice this until we tried to make use of organization_item.html in DataGM, Is that template not used anywhere in core?

[vitorbaptista]

@seanh It's weird, but it gives the correct result. The organization in that snippet is a dict returned by group_list_dictize. The packages key doesn't contain the packages themselves, but their count. It's set in https://github.com/okfn/ckan/blob/master/ckan/lib/dictization/model_dictize.py#L46

[johnmartin]

This is used in core from the main /organizations page:

https://github.com/okfn/ckan/blob/master/ckan/templates/organization/index.html#L29
https://github.com/okfn/ckan/blob/master/ckan/templates/organization/snippets/organization_list.html#L15

So this really should make it into 2.0 (this came out of the dev pull request meeting)

[vitorbaptista]

@johnmartin @seanh

I've double-checked this pr, and it seems invalid now. I've tried to reproduce the error @seanh described, but had no success:

Everything seems as expected, so I'm closing this. Please, if I missed something, reopen it. If not, delete the branch...

[seanh]

I can still reproduce this issue with the ckanext-datagm extension. The issue only occurs when organizations are shown on the front page as they are on datagm:

This branch fixes the issue but it introduces another issue which can be seen on http://datagm2.ckanhosted.com/organization now, when organizations are shown on the organization page the numbers are missing:

[vitorbaptista]

It seems like, in group_list, the 'packages' key contains the number of packages. But in group, it contains the packages themselves, while the count is moved to package_count.

There's a test to assert these behaviors in https://github.com/okfn/ckan/blob/master/ckan/tests/logic/test_action.py#L530 and https://github.com/okfn/ckan/blob/master/ckan/tests/lib/test_dictization.py#L947.

It seems we should pick one and keep using it. Probably use package_count in all places?

[seanh]

I wonder if @domoritz has touched this difference between group and group_list in #473?

@vitorbaptista When you say 'group_list' and 'group' are different, do you mean 'group_list' and 'group_show'? I'm trying to figure out why this fails on the front page but not on the group or org pages. Maybe there's a way we can workaround it without having to change the api (although the api should be fixed too if it's inconsistent)

[vitorbaptista]

@seanh I mean the results from group_list_dictize and group_dictize. The result of group_list_dictize contains a key packages with the number of packages:

# https://github.com/okfn/ckan/blob/master/ckan/lib/dictization/model_dictize.py#L45-L48
def group_list_dictize(obj_list, context,
    ...
    if obj.is_organization:
        group_dict['packages'] = query.facets['owner_org'].get(obj.id, 0)
    else:
        group_dict['packages'] = query.facets['groups'].get(obj.name, 0)

While in group_dictize, there're packages that contains the packages themselves, and package_count that contains the number of packages.

# https://github.com/okfn/ckan/blob/master/ckan/lib/dictization/model_dictize.py#L337-L346
def group_dictize(group, context):
    ...
    result_dict['packages'] = d.obj_list_dictize(
        _get_members(context, group, 'packages'),
        context)
    ...
    result_dict['package_count'] = query.run(q)['count']

[domoritz]

I wonder if @domoritz has touched this difference between group and group_list in #473?

I don't think that I have touched it.

[seanh]

What I don't understand is why, when groups are shown on the /groups page, group_list_dictize() is used but when ckanext-datagm shows groups on the front page group_dictize is used (or is it vice-versa)? Can we just change the code in ckanext-datagm so that it uses the same *-dictize() as in core, and work around this problem that way?

I agree group_dictize() group_list_dictize() should be changed to be consistent with eachother, but that'll break things so it seems more like a 2.1 thing than 2.0. @vitorbaptista always using package_count for the number of packages seems like a fair solution, but maybe look into other API actions that return counts of things, and see how they are named to see what would be most consistent. For example, I think I've seen num_packages around,

[vitorbaptista]

@seanh I guess I've found it

First, we have in ckanext-datagm's home/index.html

# https://github.com/okfn/ckanext-datagm/blob/master/ckanext/datagm/templates/home/index.html#L32-L33
{% set org_dict = h.organization_show(org_name) %}
{% snippet "organization/snippets/organization_item.html", organization=org_dict, first=first, last=last %}

h.organization_show is defined as:

# https://github.com/okfn/ckanext-datagm/blob/master/ckanext/datagm/plugin.py#L9-L11
def organization_show(name):
    return tk.get_action('organization_show')(data_dict={'id': name})

That will go to:

# https://github.com/okfn/ckan/blob/master/ckan/logic/action/get.py#L874-L883
def organization_show(context, data_dict):
    return _group_or_org_show(context, data_dict, is_org=True)

Which, finally, calls:

# https://github.com/okfn/ckan/blob/master/ckan/logic/action/get.py#L819
def _group_or_org_show(context, data_dict, is_org=False):
    ...
    group_dict = model_dictize.group_dictize(group, context)
    ...

So, even though the organization_item.html template looks the same, the object that gets passed in is different. CKAN core uses:

# https://github.com/okfn/ckan/blob/master/ckan/controllers/group.py#L157
def index(self):
    ...
    results = self._action('group_list')(context, data_dict)
    ...

[seanh]

@vitorbaptista Awesome, thanks. I've changed ckanext-datagm to use organization_list instead of organization_show, now it no longer needs to override the organization_item.html template and the orgs are rendering properly on both the front page and the /organization page: datopian/ckanext-datagm@c0301d4

I'll close this issue now as I don't think there's anything we need to fix for 2.0 anymore. But I've opened a new issue #759 to fix the inconsistent dict formats. @vitorbaptista do you want to take #759 on?

@johnmartin There's still one thing confusing me about this. Why are there two template files called organization_item.html in CKAN core? Is that intentional?