web-stories-wp
web-stories-wp copied to clipboard
Code Quality: Remove hotlinking feature flag code
Context
This feature has been enabled by default in v1.23.0.
Summary
Removes feature flags for:
videoPosterHotlinking
linkIconHotlinking
posterHotlinking
Relevant Technical Choices
To-do
User-facing changes
Testing Instructions
- [ ] This is a non-user-facing change and requires no QA
This PR can be tested by following these steps:
- Verify that hotlinking for story poster, video poster, and link icon is available and enabled and works for both contributors and admins
- Other functionality should be left unchanged.
Reviews
Does this PR have a security-related impact?
No
Does this PR change what data or activity we track or use?
No
Does this PR have a legal-related impact?
No
Checklist
- [x] This PR addresses an existing issue and I have linked this PR to it in ZenHub
- [x] I have tested this code to the best of my abilities
- [ ] I have verified accessibility to the best of my abilities (docs)
- [ ] I have verified i18n and l10n (translation, right-to-left layout) to the best of my abilities
- [ ] This code is covered by automated tests (unit, integration, and/or e2e) to verify it works as intended (docs)
- [ ] I have added documentation where necessary
- [x] I have added a matching
Type: XYZlabel to the PR
Fixes #11997
Plugin builds for 1c8b40dd0124ff08933ee60f27be9189a03ce403 are ready :bellhop_bell:!
- Download development build
- Download production build
Looks like some tests are failing now. Haven't checked closely, perhaps they need updating.
Looks like some tests are failing now. Haven't checked closely, perhaps they need updating.
expected document not to contain element, found <button data-testid="media-upload-button">Media Upload Button!</button> instead
172 | const uploadMediaButton = screen.queryByText('Media Upload Button!');
173 |
> 174 | expect(uploadMediaButton).not.toBeInTheDocument();
| ^
175 | });
Looking into it.
Tested with a user who doesn't have upload permissions ---
=====
I believe this is what we're expecting --- no upload button but can still hotlink.
Erroring on the MediaInput / ForwardRef
Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports.
Check the render method of `ForwardRef(MediaInput)`.
https://github.com/GoogleForCreators/web-stories-wp/pull/12021#discussion_r937569971
Not sure if you discussed this somewhere else that the comment was marked resolved, but since now enablePosterHotlinking is always true, wouldn't we always display this and can remove hasUploadMediaAction check as well?
It was flagged for removal but removing seems to be causing other issues -- test failures + this error https://github.com/GoogleForCreators/web-stories-wp/pull/12021#issuecomment-1196559746 thinking there's something deeper into one of the child components around the can user upload perms.
Not sure if you discussed this somewhere else that the comment was marked resolved, but since now enablePosterHotlinking is always true, wouldn't we always display this and can remove hasUploadMediaAction check as well?
It was flagged for removal but removing seems to be causing other issues -- test failures + this error #12021 (comment) thinking there's something deeper into one of the child components around the
can user uploadperms.
Hm, okay. I suppose we can leave as is then. Just by reading the code I assumed that the whole component could always displayed but canUpload should only be true in case of hasUploadMediaAction.
Hm, okay. I suppose we can leave as is then. Just by reading the code I assumed that the whole component could always displayed but canUpload should only be true in case of hasUploadMediaAction.
I'll take another look --- see if I can spot anything
@timarney That hasUploadMediaAction must go away. Otherwise contributors cannot update the story poster in the pre-publish dialog. Notice the missing button here:

Quickly addressed it in 96029c07d382d2a2214b9d3cfa0f298eb5a3af14 but there might be more tests that need updating.
@swissspidy any idea why removing that causes issues with the MediaInput (at least in the tests).
detail: Error: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports.
Check the render method of `ForwardRef(MediaInput)`.
That test renders the pre-publish modal without defining MediaUpload in the configValue in its arrange() function. See how the storyPreview.js test does it:
https://github.com/GoogleForCreators/web-stories-wp/blob/a6b3cb6047426b680db026413ca69588b13e99d0/packages/story-editor/src/components/publishModal/test/storyPreview.js#L60-L68
Now that we removed this condition, the MediaUpload component is always rendered by this test. But because it's undefined there's an error.