Take action on LbryUri.parse() and related tests after LBRY SDK mod character changes

kekkyojin commented

2020-11-12 00:48:19 +01:00

(Migrated from github.com)

Copy link

The Issue

LBRY SDK 0.84 has changed the modifier characters on URLs (lbryio/lbry-sdk@7637aa2ab6 and lbryio/lbry-sdk@e8d299d3b6).

LbryUri.parse() method on lbry-android was changing those characters for links coming from HTTPS protocol (for example, when user clicked on a link on an Android browser and chose LBRY Android to open it). Now that change is no longer needed, as accepted HTTPS urls are using ':' as the primary modifier separator, which was replaced by '#' on code as it was the expected one.

Related unit tests were about refactoring HTTPS urls to LBRY protocol urls, but part of those tests also were about the correct replacement of those primary modifier separators. It should also be changed to be consistent with LBRY SDK.

The change of the separator characters landed on SDK 0.84 but from the above lbry-sdk linked issue, it should not break the functionality of the app for a while, so this change is not strictly required to be performed between LBRY Android SDK 0.84 -when it is updated and lands on MavenCentral- and the following release.

Internal Use

Acceptance Criteria

Definition of Done

Tested against acceptance criteria
Tested against the assumptions of the user story
The project builds without errors
Unit tests are written and passing
Tests on devices/browsers listed in the issue have passed
QA performed & issues resolved
Refactoring completed
Any configuration or build changes documented
Documentation updated
Peer Code Review performed

## The Issue LBRY SDK 0.84 has changed the modifier characters on URLs (https://github.com/lbryio/lbry-sdk/commit/7637aa2ab6ee9c292f0c9ea64309143b191eaa18 and https://github.com/lbryio/lbry-sdk/commit/e8d299d3b68c4980e714da0093c3db3d0a87fa18). LbryUri.parse() method on lbry-android was changing those characters for links coming from HTTPS protocol (for example, when user clicked on a link on an Android browser and chose LBRY Android to open it). Now that change is no longer needed, as accepted HTTPS urls are using ':' as the primary modifier separator, which was replaced by '#' on code as it was the expected one. See also https://github.com/lbryio/lbry-sdk/issues/2832#issue-573031942 Related unit tests were about refactoring HTTPS urls to LBRY protocol urls, but part of those tests also were about the correct replacement of those primary modifier separators. It should also be changed to be consistent with LBRY SDK. The change of the separator characters landed on SDK 0.84 but from the above lbry-sdk linked issue, it should not break the functionality of the app for a while, so this change is not strictly required to be performed between LBRY Android SDK 0.84 -when it is updated and lands on MavenCentral- and the following release. ## Internal Use ### Acceptance Criteria 1. 2. 3. ### Definition of Done - [ ] Tested against acceptance criteria - [ ] Tested against the assumptions of the user story - [ ] The project builds without errors - [ ] Unit tests are written and passing - [ ] Tests on devices/browsers listed in the issue have passed - [ ] QA performed & issues resolved - [ ] Refactoring completed - [ ] Any configuration or build changes documented - [ ] Documentation updated - [ ] Peer Code Review performed

kauffj commented

2021-05-24 20:46:12 +02:00

(Migrated from github.com)

Copy link

App must handle both : and # and ship in advance of wallet server upgrade. After upgrade, code referencing # can be removed.

App must handle both `:` and `#` and ship in advance of wallet server upgrade. After upgrade, code referencing `#` can be removed.

No Branch/Tag specified