Fix local media id collisions

[mkevins]

Fixes #11239

Related PR:

gutenberg-mobile: wordpress-mobile/gutenberg-mobile#1800

This PR aims to prevent local media ids from colliding with remote ids during ongoing uploads. The issue is described in more detail here: wordpress-mobile/gutenberg-mobile#1610 (comment). The approach used in this PR is to negate the local media id over the bridge so local ids are represented as negative integers to the JavaScript code. All interactions over the bridge involving these local ids (primarily media upload events) will negate the local id. In this way, the media models and database logic used within WordPress-Android can remain unaffected, and the React Native (JavaScript) logic will avoid id collisions.

Thoughts on an alternative approach

The approach in this PR is not ideal, as it introduces a bit of complexity. Another idea to consider is to re-detect whether a particular media item is local or remote by pattern matching the url on the JS side. If this is possible, there may be another solution to consider. I will explore this to see if a robust solution is possible via that route.

To test

tbd

PR submission checklist:

• I have considered adding unit tests where possible.

• I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

[peril-wordpress-mobile]

	Warnings
⚠️	PR is missing at least one label.

Generated by 🚫 dangerJS

[jkmassel]

We're freezing 14.2 today, so this PR is being bumped to 14.3. If you'd like it to ship with 14.2, please merge it into release/14.2 and ping me – I'll be happy to cut a new beta release.

[peril-wordpress-mobile]

You can test the changes on this Pull Request by downloading the APK here.

[oguzkocer]

We're freezing 14.3 today, so this PR is being bumped to 14.4. If you'd like it to ship with 14.3, please merge it into release/14.3 and ping me – I'll be happy to cut a new beta release.

[oguzkocer]

We're freezing 14.4 today, so this PR is being bumped to 14.5. If you'd like it to ship with 14.4, please merge it into release/14.4 and ping me – I'll be happy to cut a new beta release.

---

@mkevins It looks like this PR has been opened more than a month ago without any significant updates to it. The issue that it's referring to is closed. Could you please follow up on this PR?

[mkevins]

The issue that it's referring to is closed. Could you please follow up on this PR?

Thanks for noticing that. The underlying issue has been moved to its own issue, but the link in the issue description for this PR had not been updated to point to that. I've updated it now.

Regarding the milestone, I'm unsure whether this PR should have one yet, since it's only in a draft state. Would it make more sense to remove the milestone for now to reduce the "chore" of constantly bumping it?

[oguzkocer]

Regarding the milestone, I'm unsure whether this PR should have one yet, since it's only in a draft state. Would it make more sense to remove the milestone for now to reduce the "chore" of constantly bumping it?

@mkevins Sounds good to me. It'd be great if you can add the milestone when it's marked as ready, so it'll show up in the release wrap up.

Actually, it might make sense for us not to run the milestone check for draft PRs, but I don't know if that'd be possible or something easy to do. cc @jkmassel

[jkmassel]

This is a good idea - I’ll add it to our backlog.

One workaround we have for other projects is a “Future” milestone that’s for things like this, where we aren’t sure where they’ll land.

[mkevins]

The related gutenberg-mobile PR has been closed since the monorepo has landed, and the changes will be necessary on the bridge in the gutenberg repository.

[justtwago]

Closed according to this proposal: pcdRpT-4Oj-p2