-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Apple App Store bridge fix #4516
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Pull request artifacts
last change: Monday 2025-04-21 18:16:57 |
bridges/AppleAppStoreBridge.php
Outdated
return 'https://apps.apple.com/' . $country . '/app/id' . $id; | ||
$id = $this->getInput('id'); | ||
$country = $this->getInput('country'); | ||
return "https://apps.apple.com/{$country}/app/id{$id}"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls choose an alternative way to interpolate string, e.g. sprintf
bridges/AppleAppStoreBridge.php
Outdated
$json = json_decode($script->innertext, true); | ||
return $json['data']; | ||
if ($this->getInput('debug')) { | ||
error_log("[AppleAppStoreBridge] $message"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls dont use error_log
.
use:
$this->logger->info('hello world')
$this->logger->warning('hello world')
$this->logger->error('hello world')
bridges/AppleAppStoreBridge.php
Outdated
|
||
$html = getSimpleHTMLDOM($url); | ||
|
||
if (!$html) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will never happen because return value is always \simple_html_dom
, or it throws exception
throw new \Exception("JWT token not found in page content"); | ||
} | ||
|
||
try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
put as little code as possible in the try clause
bridges/AppleAppStoreBridge.php
Outdated
try { | ||
$decoded_content = urldecode($meta->content); | ||
$this->debugLog("Found meta tag content"); | ||
$decoded_json = json_decode($decoded_content, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See Json::decode
bridges/AppleAppStoreBridge.php
Outdated
|
||
|
||
$content = getContents($url, $headers); | ||
if (!$content) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getContents
returns string|Response
, most commonly a string
bridges/AppleAppStoreBridge.php
Outdated
default: | ||
$os = self::PLATFORM_MAPPING[$platform]; | ||
return $data['attributes']['platformAttributes'][$os]['versionHistory']; | ||
try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code does not need to be wrapped in a try catch
bridges/AppleAppStoreBridge.php
Outdated
|
||
if (isset($data['attributes']['platformAttributes'][$platform_key]['versionHistory'])) { | ||
return $data['attributes']['platformAttributes'][$platform_key]['versionHistory']; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should work:
$version = $data['attributes']['platformAttributes'][$platform_key]['versionHistory'] ?? null
bridges/AppleAppStoreBridge.php
Outdated
$this->debugLog("Getting data for " . $this->getInput('p') . " app"); | ||
$data = $this->getAppData(); | ||
|
||
list($name, $author) = $this->extractAppDetails($data); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls dont use list()
. I think php has proper lang syntax for this unpacking.
Something like:
[$name, $auth] = $this->extractAppDetails($data);
lint |
Sorry, I originally wrote the script in Python and don't know much about PHP. |
Update to new website structure.