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

Change docs of create & update methods of bugzilla_component #162

Closed
wants to merge 1 commit into from

Conversation

zkl94
Copy link
Collaborator

@zkl94 zkl94 commented Oct 14, 2015

the docs for the original create and update method doesn't provide the correct request formats. it also doesn't coincide with the corresponding test cases.

@erichuanggit
Copy link
Collaborator

could we think about auto doc but not provide by manual?

@zkl94
Copy link
Collaborator Author

zkl94 commented Oct 15, 2015

@erichuanggit I will try to figure that out

@zkl94
Copy link
Collaborator Author

zkl94 commented Oct 15, 2015

@erichuanggit I think we can't automate the doc generation by using tools provided in common/renderers.py. The macros provide two options for dealing with serialisers:

  1. %(SERIALIZER)s will be replaced by a code block with details about
    serialiser
  2. %(WRITABLE_SERIALIZER)s will do the same, but without read-only fields

they either take all the fields or take all the writable fields. As you can see in the BugzillaComponentSerializer, all fields are not marked read_only and parent_pk field is missing. To set the fields needed to replace the macro we should simply customise it. Such examples can be seen in GlobalComponentViewSet, ReleaseComponentViewSet and so on.

@simozhan
Copy link
Contributor

The thing is parent bugzillacomponent name can be duplicated. So we use parent_pk in update API.
But for filters, we use the name as query parameter. So is it ok to change the query parameter to parent_pk too?

@lubomir
Copy link
Member

lubomir commented Oct 15, 2015

The only way to do it automatically would be to use the writable_doc_format attribute on the serializer. However, it only works for serializer fields or nested serializers now, not top-level serializer. It would have to be implemented in the renderer.

@zkl94
Copy link
Collaborator Author

zkl94 commented Oct 16, 2015

@simozhan

  1. using parent_component: when people wants to search for a bugzilla_component as the child of some other bugzilla_component, he will just need to search for once. this is like fuzzy search. It is like cross join in SQL. It is convenient but when many duplicate names exist it will cause trouble and there is no way to do exact search. The possible result is either for someone to quickly find the result he needs in the result set when they are few duplicate names or almost never find it because of the mess of m*n results when they are many duplicate names. So the problem is whether there will be many duplicate names for bugzilla_component?
  2. using parent_pk: when people wants to search for a bugzilla_component as the child of some other bugzilla_component, he will have to first search for the id of the parent bugzilla_component and then search again with that id for the child bugzilla_component. It is like two single table queries in SQL. this is more like exact search but it will take two steps to find the bugzilla_component needed. But the possible result of this is that it will be easier for cilents to find what.

so it seems both are needed. Each of these have its advantage and disadvantage. Because I don't know the exact information of how many bugzilla_components there are and whether they will be many duplicate names, I don't know the answer for this one.

@zkl94
Copy link
Collaborator Author

zkl94 commented Oct 16, 2015

@lubomir so maybe we can use this fix temporarily and create another task for the renderer?

@lubomir
Copy link
Member

lubomir commented Oct 16, 2015

I'm all for merging this patch. Cleaning up later is a possibility, but not a hard requirement.

@zkl94 zkl94 closed this Oct 16, 2015
@zkl94 zkl94 deleted the PDC-1067 branch October 16, 2015 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants