arthur-shaw / susoapi

R interface for Survey Solutions' APIs
https://arthur-shaw.github.io/susoapi/
Other
9 stars 5 forks source link

`get_assignment` function fetches assignments from all questionnaires if version is not defined. #27

Closed ashwinikalantri closed 2 years ago

ashwinikalantri commented 2 years ago

In the get_assignment function, the expected function if qnr_version argument is not defined should be to get all assignments of the questionnaire defined in the qnr_id argument. Instead, this fetches all the assignments from all the questionnaires in the defined workspace.

petbrueck commented 2 years ago

Just stumbled over the same issue.

Current Workaround Define qnr_version

Suggestion

If I see it correctly, the following ifelse condition used in get_assignment_count and get_assignment_batch returns "" if no qnr_version is defined:

    # form the questionnaire ID as QuestionnaireID$Version
    qnr_id_full <- ifelse(
        test = qnr_id != "" & qnr_version != "",
        yes = paste0(qnr_id, "$", qnr_version),
        no = ""
    )

which then leads to requesting Assignments of all questionnaires.

Since the REST API /api/v1/assignments endpoint is used, which requires Questionnaire Id in format of {QuestionnaireId}${Version}, I'd suggest to either

Having the feature that the function scans for all versions on the server based on qnr_version in case no version is specified seems over the top to me?

arthur-shaw commented 2 years ago

I like @petbrueck 's first proposed solution: checking that qnr_version is provided and is valid. @ashwinikalantri , @petbrueck , would that be OK?

Another complementary question: do you see any compelling reason to migrate to GraphQL for fetching assignment details? My answer is "no". The REST API provides a few things GraphQL doesn't: number of interviews collected, update date, whether expect audio recording, questionnaire ID and version, and perhaps other things I've missed. The GraphQL version contains details on the calendar events. But I'm very open to other views.

Note: I'm currently preparing a new release of {susoapi} that will tackle a few big tickets (e.g., #24 , #26 , #23 ) and bug fixes--among them this one.

ashwinikalantri commented 2 years ago

I agree that checking qnr_version will ensure not getting unexpected assignments.

On the other hand, getting all assignments of a questionnaire using a single function could be useful. Otherwise we have to loop the function for all the versions and then combine them. It may be better to have the function handle this.

arthur-shaw commented 2 years ago

I'll think about this a bit more, but here's my current inclination: