Jetpack: WordAds API functions return doesn't match docs

[rclations]

Steps to reproduce the issue

There are several functions in modules/wordads/php/api.php that are documented as returning a boolean, but actually return a string of 1 or 0. We should either change the output, or change the docs, and test thoroughly to make sure there are no unintended side effects.

    /**
     * Returns status of WordAds approval.
     * @return boolean true if site is WordAds approved
     *
     * @since 4.5.0
     */
    public static function is_wordads_approved() {
        if ( is_null( self::$wordads_status ) ) {
            self::get_wordads_status();
        }
        return self::$wordads_status['approved'] ? '1' : '0';
    }
    /**
     * Returns status of WordAds active.
     * @return boolean true if ads are active on site
     *
     * @since 4.5.0
     */
    public static function is_wordads_active() {
        if ( is_null( self::$wordads_status ) ) {
            self::get_wordads_status();
        }
        return self::$wordads_status['active'] ? '1' : '0';
    }
    /**
     * Returns status of WordAds house ads.
     * @return boolean true if WP.com house ads should be shown
     *
     * @since 4.5.0
     */
    public static function is_wordads_house() {
        if ( is_null( self::$wordads_status ) ) {
            self::get_wordads_status();
        }
        return self::$wordads_status['house'] ? '1' : '0';
    }
    /**
     * Returns whether or not this site is safe to run ads on.
     * @return boolean true if ads shown not be shown on this site.
     *
     * @since 6.5.0
     */
    public static function is_wordads_unsafe() {
        if ( is_null( self::$wordads_status ) ) {
            self::get_wordads_status();
        }
        return self::$wordads_status['unsafe'] ? '1' : '0';
    }

[rclations]

I can haz grammars. 😃

[rohanjulka19]

Hi , I want to work on this issue is that ok ?

[rclations]

Hey @rohanjulka19, of course!

[rohanjulka19]

I think its better to just change the documentation because changing other way around can be problematic?

[omkarscode]

that are documented as returning a boolean, but actually return a string of 1 or 0

I think we should change the code to reflect what the documentation is saying because if I understand this correctly - returning a boolean value makes more sense and is the intended design.

I feel boolean is the right way to go because these are all returning a status which is yes or no.

And we will surely need to test for side-effects :)

The fix should be changing ? '1' : '0'; to ? 1 : 0;. Correct me if I'm wrong! :)

[kraftbj]

For bool, I'd just use true and false. Changing to 1 and 0 would be an int and still need the type juggling.

[andyjdavis]

I have raised a PR containing a possible solution for this. Feedback welcome :)

https://github.com/Automattic/jetpack/pull/18823