Moodle

Validate_parameters should return an array without the optional parameter on update operation

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: 2.0
  • Fix Version/s: 2.0
  • Component/s: Web Services
  • Labels:
    None
  • Affected Branches:
    MOODLE_20_STABLE
  • Fixed Branches:
    MOODLE_20_STABLE

Description

When a ws client calls update_users(), the validate_parameters function will return all optional attributs with their default values (even if the ws client send an object without any optional attribut).
This implies that if a ws client wants to update a user, the ws client must retrieve the user with all his optional values first. Then the ws client must send the complete user object in the update_users() call (with the optional attributs set to the previously retrieved values, if these values are the same).
If the ws client doesn't send a full user object, the missing optional attributs will be overwritten by their default in the description.

Issue Links

This issue has a non-specific relationship to:
MDL-12886 Web service infrastructure Critical Closed

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Jerome Mouneyrac added a comment -

I attached a patch after talking about it with Martin.

Show
Jerome Mouneyrac added a comment - I attached a patch after talking about it with Martin.
Hide
Permalink
Petr Škoda (skodak) added a comment -

I do not like this approach, I understand why you want it but it imho encourages very bad coding practices.

1/ If you want to use only some parameters then you might add a new array parameter to the actual function call and specify a full list - like new function call that updates user record and the last parameter is a list of optional params that are used
2/ or we could extend the array definitions and make it : required/optional/somethingelse and the validation itself would not add them, but the point is it would be part of definition and happen only inside the validate_parameters() - adding new parameter for validate_parameters () is breaking the original design imo

Show
Petr Škoda (skodak) added a comment - I do not like this approach, I understand why you want it but it imho encourages very bad coding practices. 1/ If you want to use only some parameters then you might add a new array parameter to the actual function call and specify a full list - like new function call that updates user record and the last parameter is a list of optional params that are used 2/ or we could extend the array definitions and make it : required/optional/somethingelse and the validation itself would not add them, but the point is it would be part of definition and happen only inside the validate_parameters() - adding new parameter for validate_parameters () is breaking the original design imo
Hide
Permalink
Petr Škoda (skodak) added a comment -

also please note that things like this need to be described in inline docs too, with the patch it would be hard to generate it automatically...

Show
Petr Škoda (skodak) added a comment - also please note that things like this need to be described in inline docs too, with the patch it would be hard to generate it automatically...
Hide
Permalink
Martin Dougiamas added a comment -

Yeah, I can see the point about docs.

So three defines (which I far prefer to the "false" there now anyway)

VALUE_REQUIRED - if the parameter is not supplied, there is an error
VALUE_OPTIONAL - if the parameter is not supplied, then the param has no value
VALUE_DEFAULT - if the parameter is not supplied, then the default value is used

OK?

Show
Martin Dougiamas added a comment - Yeah, I can see the point about docs. So three defines (which I far prefer to the "false" there now anyway) VALUE_REQUIRED - if the parameter is not supplied, there is an error VALUE_OPTIONAL - if the parameter is not supplied, then the param has no value VALUE_DEFAULT - if the parameter is not supplied, then the default value is used OK?
Hide
Permalink
Petr Škoda (skodak) added a comment -

yes, sounds good Martin +1

Show
Petr Škoda (skodak) added a comment - yes, sounds good Martin +1
Hide
Permalink
Jerome Mouneyrac added a comment -

I also created 2 other DEFINE values:
NULL_NOT_ALLOWED
NULL_ALLOWED

Show
Jerome Mouneyrac added a comment - I also created 2 other DEFINE values: NULL_NOT_ALLOWED NULL_ALLOWED

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.