-
Notifications
You must be signed in to change notification settings - Fork 140
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
Make organization source required #5137
base: report-BP-id-to-bugsnag
Are you sure you want to change the base?
Make organization source required #5137
Conversation
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
We detected some changes at packages/*/src and there are no updates in the .changeset. |
Coverage report
Test suite run success1994 tests passing in 900 suites. Report generated by 🧪jest coverage report action from e746586 |
9ebb294
to
e746586
Compare
Differences in type declarationsWe detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:
New type declarationsWe found no new type declarations in this PR Existing type declarationspackages/cli-kit/dist/public/node/monorail.d.ts@@ -16,6 +16,7 @@ export interface Schemas {
env_plugin_installed_all?: Optional<string>;
};
public: {
+ business_platform_id?: Optional<number>;
partner_id?: Optional<number>;
command: string;
project_type?: Optional<string>;
packages/cli-kit/dist/public/node/plugins.d.ts@@ -16,7 +16,7 @@ import { Config, Interfaces } from '@oclif/core';
* @returns A dictionary of plug-in names to the response from the hook.
*/
export declare function fanoutHooks<TPluginMap extends HookReturnsPerPlugin, TEvent extends string & keyof TPluginMap>(config: Interfaces.Config, event: TEvent, options: TPluginMap[typeof event]['options'], timeout?: number): Promise<Partial<TPluginMap[typeof event]['pluginReturns']>>;
-type AppSpecificMonorailFields = PickByPrefix<MonorailEventPublic, 'app_', 'project_type' | 'api_key' | 'partner_id'> & PickByPrefix<MonorailEventPublic, 'cmd_extensions_'> & PickByPrefix<MonorailEventPublic, 'cmd_scaffold_'>;
+type AppSpecificMonorailFields = PickByPrefix<MonorailEventPublic, 'app_', 'project_type' | 'api_key' | 'partner_id' | 'business_platform_id'> & PickByPrefix<MonorailEventPublic, 'cmd_extensions_'> & PickByPrefix<MonorailEventPublic, 'cmd_scaffold_'>;
type AppSpecificSensitiveMonorailFields = PickByPrefix<MonorailEventSensitive, 'app_'>;
export interface HookReturnsPerPlugin extends HookReturnPerTunnelPlugin {
public_command_metadata: {
|
WHY are these changes introduced?
Makes the
source
field required in theOrganization
type to ensure consistent handling of organization sources across the codebase. This prevents potential undefined behavior when dealing with organization sources, and - most relevant here - allows us to be sure we can report analytics with the correct org ID field.WHAT is this pull request doing?
Makes the
source
field non-optional in theOrganization
interface and updates all test files to include the requiredsource
field in organization objects. The changes ensure that organizations always have a defined source, eitherBusinessPlatform
orPartners
.How to test your changes?
Not much to test per se, I'd suggest tophatting with downstream PRs. All we're doing is being more strict about field requirements.
Measuring impact
Checklist