Show incompatible error for missing platform URLs#3419
Show incompatible error for missing platform URLs#3419kumar303 merged 2 commits intomozilla:masterfrom
Conversation
kumar303
force-pushed
the
platform-install-bug-3266
branch
from
325509f to
fa8010b
Compare
Codecov Report
@@ Coverage Diff @@
## master mozilla/addons-frontend#3419 +/- ##
==========================================
+ Coverage 95.72% 95.72% +<.01%
==========================================
Files 175 175
Lines 3366 3371 +5
Branches 672 674 +2
==========================================
+ Hits 3222 3227 +5
Misses 124 124
Partials 20 20
Continue to review full report at Codecov.
|
tofumatt
left a comment
tofumatt
left a comment
There was a problem hiding this comment.
r+wc, I'm not sure about the theme test and I think you could use the existing dispatchClientMetadata helper. Looks good otherwise, thanks!
| return { compatible: false, reason: INCOMPATIBLE_NO_OPENSEARCH }; | ||
| } | ||
|
|
||
| // Even if an extension's version is marked compatible, |
There was a problem hiding this comment.
These are the edge cases that make AMO so special 😆
| let store; | ||
|
|
||
| beforeEach(() => { | ||
| store = createStore().store; |
There was a problem hiding this comment.
This could use dispatchClientMetadata for a more accurate/complete "default" store.
There was a problem hiding this comment.
This component relies on lang and userAgent so I wanted each test to call dispatchClientMetadata() instead since the values are important. I can add a comment to this if it's helpful though.
| lang: 'en-GB', | ||
| reason: INCOMPATIBLE_NOT_FIREFOX, | ||
| userAgentInfo: { browser: { name: 'Chrome' }, os: {} }, | ||
| _dispatchClientMetadata({ |
There was a problem hiding this comment.
Not sure why this was made when we could use the existing dispatchClientMetadata helper.
There was a problem hiding this comment.
With this helper I don't have to pass store into the function every time
| }); | ||
| }); | ||
|
|
||
| it('allows non-extensions to have mismatching platform files', () => { |
There was a problem hiding this comment.
Does this ever happen? I didn't think themes could/did set platform.
There was a problem hiding this comment.
I'm not sure but since this code is only working around a bug in extensions I wanted to limit it there until we know we need it elsewhere.
3 participants
Fixes mozilla/addons#10829
Previously when an add-on declared itself compatible with a platform yet was missing a platform file, we just let the user try to install it. This resulted in a console error because there was no install URL. This patch shows an incompatible error banner and disables the install button instead.