closes https://linear.app/tryghost/issue/ENG-739
- `props.innerRef` in `<IFrame>` was inadvertently assumed to always exist, sometimes throwing an error on render when certain popups like the profile settings modal were opened resulting in the app crashing and the whole comments block disappearing
- added a guard to ensure the ref exists before trying to set it
- updated tests so the profile modal route is tested
closes https://linear.app/tryghost/issue/ENG-721
ref https://linear.app/tryghost/issue/ENG-708
Comments-UI loads `/ghost/admin-frame/` in an iframe to check if a Staff User is authenticated in order to show moderation options. That iframe request loads a HTML page which in turn contains a script that fires off an API request that attempts to fetch the logged-in user details, resulting in a 403 "error" showing up when not authenticated. In the vast majority of cases there will be no staff user authenticated so lots of extra requests and "errors" are seen unnecessarily.
- adjusted the `/ghost/auth-frame/` endpoint to check if the request contains an Admin session cookie
- if it does, continue as before with rendering the HTML page so the script is loaded
- if it doesn't, return an empty 204 response avoiding the script request and subsequent 403-generating API request
- eliminates the 403 error being generated for all typical visitor traffic, the error should only be seen when an Admin was previously logged in but their cookie is no longer valid (either from logging out, or going past the 6month validity period)
no issue
- defers loading of `admin-auth` frame and it's API request until we are displaying some comments
- defers the frame load and API request until the comments box is scrolled into view
- eliminates the requests altogether for posts that have no comments
closes ENG-711
When an Admin is authenticated in Comments-UI we only add moderation options to the displayed comments so we don't need to pre-emptively load the `admin-auth` iframe and make the `/ghost/api/admin/users/me/` request until some comments are actually visible.
- used `state.comments.length` property to defer rendering of the admin auth frame until comments have been fetched (after box is scrolled into view) and the count is > 0
no issue
- switches post browse requests to`/members/api/comments/post/:post_id/` to enable better cache bucketing
and invalidation
- removes `order` param from browse and replies requests
closes ENG-681
There's no need to provide an `order` param with every request in Comments-UI if the API has default ordering that matches our requirements. The order param makes logs more noisy/harder to read than they need to be so we want to get rid of it.
- modified comments API input serializer to add a default order param to the browse and replies endpoints when none is provided
- removed order param from the requests that Comments-UI makes
refs https://linear.app/tryghost/issue/ENG-676/
Now we have the case that there is no filter param, the simple string
approach fails. Instead we build up a URLSearchParams object which
makes it easier to handle conditional params & stringify it at the end.
closes ENG-678
The comments block is typically shown at the bottom of a post so it doesn't make sense to eagerly fetch comments from the API when we don't know if the comments block will even be viewed. By lazy-loading the data only when the comments block comes into view we can reduce both data usage for visitors and load on the site.
- uses IntersectionObserver API to delay comments app initialisation until the comments block has scrolled into view
- updated all iframe-related components to forward a `ref` so we can use the `<iframe>` element reference inside the `App` component
refs https://linear.app/tryghost/issue/ENG-673
This means that the initial load of comments can be cached for everyone.
We also improve the timestamp which is used when fetching future comments,
instead of using a locally generated timestamp, we use the created_at of
the first comment loaded, this drastically improved the likelyhood that the
timestamp will be the same, meaning that it will hit the cache.
no ref
- includes run of `eslint --fix` to update changed classname ordering in Tailwind
---------
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Kevin Ansfield <kevin@lookingsideways.co.uk>
fixes GRO-34
fixes GRO-33
This is a revision of a previous commit, that broke the browser tests
because changes in the data generator (requiring bookshelf had side
effects).
This adds a new way to run all tests with enforced numeric ObjectIDs.
These numeric ids cause issues if they are used withing NQL filters. So
they surface tiny bugs in our codebase.
You can run tests using this option via:
NUMERIC_IDS=1 yarn test:e2e
Removed some defensive logic that could be explained by this discovered
issue.
fixes GRO-34
fixes GRO-33
This also adds a new way to run all tests with enforced numeric ObjectIDs.
These numeric ids cause issues if they are used withing NQL filters. So they
surface tiny bugs in our codebase.
You can run tests using this option via:
NUMERIC_IDS=1 yarn test:e2e
Also removed some defensive logic that could be explained by unquoted ids.
refs https://github.com/TryGhost/DevOps/issues/104
- `info` is very verbose as it prints all files and their sizes
- we often don't care about this, so we can do away with `info` in favor
of `warn`
refs https://vitejs.dev/config/build-options.html#build-reportcompressedsize
- this should make building a little bit quicker because it doesn't have
to calculate the gzip size (I don't think we're likely to hit this
because we don't have large projects, but it's still nice to clean up
the output)
