Skip to content
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

Added @tryghost/metrics-server as a dependency #21329

Closed
wants to merge 12 commits into from

Conversation

cmraible
Copy link
Collaborator

@cmraible cmraible commented Oct 16, 2024

no issue

  • I apparently never added @tryghost/metrics-server as a dependency to ghost/core/package.json. It worked in most cases as a 'Ghost dependency' — yarn installs all node_modules in a flat structure, so even though it wasn't a dependency in package.json, it still resolved to the correct package, as long as the typescript packages were all built first.
  • This passed CI because we explicitly run ts:build on all packages before running tests, and it worked in production because we build the TS packages as part of the docker build. However, when trying to run tests locally, it would sometimes fail unless you explicitly ran nx run-many -t build:ts at the top level before running the tests.
  • Adding it as a dependency in package.json fixes this problem.

github-actions bot and others added 12 commits October 16, 2024 12:16
- we need this to happen before running the setup script because TS
  dependencies won't be compiled at this point
)

closes TryGhost#16748 

The members/:member_id/signin_urls endpoint currently only does
cookie-based authentication. When TryGhost#21249 is merged, turning on 2FA is
going to break any 3rd party processes that use it (including my social
sign-in offering).

This patch gives admin API keys 'read' permission on this endpoint, and
enables 3rd party processes to handle user logins the right way, instead
of via a staff member's email/password.

Migration included.  Feedback appreciated.

I have the wrong name on my migration. I can see it doesn't follow the
naming convention, but I'm not sure how the names are generated.

---------

Co-authored-by: Michael Barrett <[email protected]>
…#19676)

- this helps catch test failures that are due to us writing timezone dependent code

Co-authored-by: Daniel Lockyer <[email protected]>
no ref

Stubbed expected test errors. In general, we should be expecting these
errors in the tests as we write them as that is the expected behavior
(or that behavior should change).
…host#21325)

no ref

When running tests, occasionally we'll see some varying sort in the
members api response because members are generally all created with the
same timestamp. While `ObjectId` should be progressive, and our defalut
sort is `ORDER BY created_at desc, id desc`, we still would sometimes
see issues. This ought to remove any flakiness.
ref https://linear.app/tryghost/issue/ONC-387/

With some recent changes, we added validation to unsubscribe URLs to verify the source, allowing us to cut down on spam and improving security, as the underlying key could be re-generated should the need arise. This had the side effect of making unsubscribe URLs difficult to reconstruct when using third-party/downstream integrations, such as ActiveCampaign, which fills a gap in the current Ghost feature set.

Now any authenticated query to `/api/members` will return an `unsubscribe_url` field that can be used directly.
@github-actions github-actions bot added the migration [pull request] Includes migration for review label Oct 16, 2024
Copy link
Contributor

It looks like this PR contains a migration 👀
Here's the checklist for reviewing migrations:

General requirements

  • Satisfies idempotency requirement (both up() and down())
  • Does not reference models
  • Filename is in the correct format (and correctly ordered)
  • Targets the next minor version
  • All code paths have appropriate log messages
  • Uses the correct utils
  • Contains a minimal changeset
  • Does not mix DDL/DML operations
  • Tested in MySQL and SQLite

Schema changes

  • Both schema change and related migration have been implemented
  • For index changes: has been performance tested for large tables
  • For new tables/columns: fields use the appropriate predefined field lengths
  • For new tables/columns: field names follow the appropriate conventions
  • Does not drop a non-alpha table outside of a major version

Data changes

  • Mass updates/inserts are batched appropriately
  • Does not loop over large tables/datasets
  • Defends against missing or invalid data
  • For settings updates: follows the appropriate guidelines

@cmraible cmraible changed the title Add metrics server dep Added @tryghost/metrics-server as a dependency Oct 16, 2024
@cmraible cmraible closed this Oct 16, 2024
@cmraible cmraible deleted the add-metrics-server-dep branch October 16, 2024 19:24
@cmraible cmraible restored the add-metrics-server-dep branch October 16, 2024 19:24
@cmraible
Copy link
Collaborator Author

Somehow branch got messed up, closing in favor of #21330

@cmraible cmraible deleted the add-metrics-server-dep branch October 16, 2024 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
migration [pull request] Includes migration for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants