Page MenuHomePhabricator

Consistent consumption of announcements feed endpoint
Closed, ResolvedPublic

Description

The iOS app implementation differs from the Android app when it consumes the announcements feed endpoint.

https://gerrit.wikimedia.org/r/#/c/327224/4..5 and other patches brought up several inconsistencies:

  • text - Android app expects HTML-formatted text, iOS app cannot handle HTML tags
  • image. The iOS app uses image_url instead.
  • caption_HTML - must be <p>-wrapped for iOS, must NOT be for Android

We should try to be more consistent so we don't run into copy&paste issues down the road.

Event Timeline

Bummer. With the new Android platform AndroidAppV2 in T188431 we missed the opportunity to change the image field to image_url in Android.
I think when iOS comes up with something like this (iOSAppV2) we could at least change the value for type from donation to fundraising. We might also consider changing image_url to image for iOS so that both platforms are at least consistent. (Although I admit that I like image_url better.)

  • type value should be fundraising. iOS expects donation

Is this (still) true? As far as I can tell, there's nothing relying on either one or the other in the iOS codebase. Personally I prefer fundraising.

Bummer. With the new Android platform AndroidAppV2 in T188431 we missed the opportunity to change the image field to image_url in Android.

No time like the present. I'll put in a quick Android patch.

Change 460862 had a related patch set uploaded (by Mholloway; owner: Mholloway):
[apps/android/wikipedia@master] Change serialized name of announcement imageUrl field to "image_url"

https://gerrit.wikimedia.org/r/460862

@JMinor Could the iOS app easily be updated to handle HTML-formatted announcement card body text?

Please see also my question above (T153224#4587613) about the fundraising / donation announcement type.

Change 460862 merged by jenkins-bot:
[apps/android/wikipedia@master] Change serialized name of announcement imageUrl field to "image_url"

https://gerrit.wikimedia.org/r/460862

@Mholloway the iOS app could be updated, but would be a somewhat significant undertaking due to the bugginess of iOS's built in HTML --> attributed string handling. Is there a limited set of HTML tags used or should it be able to handle pretty much any arbitrary HTML?

@JoeWalsh A limited set is all we need. We're not doing anything fancy, only basic formatting. The only tags we've used for Android to date are <b> and <br>.

I'll also note here another discrepancy I found after some git-spelunking, which is that the iOS app requires that the caption_HTML text be <p>-wrapped, while for Android it should not be. We should reconcile that, too.

Mholloway raised the priority of this task from Low to Medium.Sep 18 2018, 7:44 AM

We should be able to reconcile this while making changes for https://phabricator.wikimedia.org/T204821

I have a work in progress patch that addresses the following:

  • captionHTML no longer needs to be wrapped in a paragraph tag,
  • <b><u><i> and <a> tags can be handled in the main text - should the naming of this property be message_HTML to match caption_HTML?
  • image and image_url are both handled

I couldn't find anything indicating the app was dependent on type being donation instead of fundraising - @Mholloway was there something in particular you found that indicated that?

Also, for handling <br> tags, should any newline characters be ignored?

@bearND @Mholloway I ended up making some consolidation changes for the fundraising announcement to test with the iOS app locally, so I submitted a patch:

https://gerrit.wikimedia.org/r/#/c/mediawiki/services/mobileapps/+/473248/

Feel free to reject if it's not the direction you were planning on going with it, but figured I'd put it up in case it was helpful.

@bearND @Mholloway I ended up making some consolidation changes for the fundraising announcement to test with the iOS app locally, so I submitted a patch:

Thanks!

I couldn't find anything indicating the app was dependent on type being donation instead of fundraising - @Mholloway was there something in particular you found that indicated that?

I don't know how that was determined originally. When I looked a couple of months ago (and again now), as far as I can tell, the iOS app doesn't care about the announcement's type at all. I'll just be bold and update the description to remove this.

Mholloway assigned this task to JoeWalsh.

Created https://www.mediawiki.org/wiki/Specs/Announcements/0.3.0 reflecting the recent iOS updates. Thanks @JoeWalsh for your work on this!