From cfc188dde88882f5ee2941fe42b2848b4b368635 Mon Sep 17 00:00:00 2001
From: Jessica Janiuk <jessica.janiuk@gmail.com>
Date: Tue, 7 May 2024 10:14:03 -0400
Subject: [PATCH] fix(ng-dev): clarify isolate primitives validation rules

This updates the isolate primitives validation to ensure that it is clear that once primitives code has been merged, PRs can include FW code as long as primitives code is present in the PR. In that case, those changes are related, which is fine. Pure FW PRs should not be merged in this case.
---
 .github/local-actions/branch-manager/main.js     |  2 +-
 ng-dev/pr/common/test/common.spec.ts             | 13 +++++++++++++
 .../validation/assert-isolated-separate-files.ts | 16 ++++++++++++++--
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/.github/local-actions/branch-manager/main.js b/.github/local-actions/branch-manager/main.js
index f25c747f4..ca256fdf1 100644
--- a/.github/local-actions/branch-manager/main.js
+++ b/.github/local-actions/branch-manager/main.js
@@ -68106,7 +68106,7 @@ var Validation4 = class extends PullRequestValidation {
     if (diffStats.separateFiles > 0 && !hasSeparateSyncFiles) {
       throw this._createError(`This PR cannot be merged as Shared Primitives code has already been merged. Primitives and Framework code must be merged and synced separately. Try again after a g3sync has finished.`);
     }
-    if (diffStats.files > 0 && hasSeparateSyncFiles) {
+    if (diffStats.files > 0 && diffStats.separateFiles === 0 && hasSeparateSyncFiles) {
       throw this._createError(`This PR cannot be merged as Angular framework code has already been merged. Primitives and Framework code must be merged and synced separately. Try again after a g3sync has finished.`);
     }
   }
diff --git a/ng-dev/pr/common/test/common.spec.ts b/ng-dev/pr/common/test/common.spec.ts
index 306814674..4e8351900 100644
--- a/ng-dev/pr/common/test/common.spec.ts
+++ b/ng-dev/pr/common/test/common.spec.ts
@@ -212,6 +212,19 @@ describe('pull request validation', () => {
       const results = await assertValidPullRequest(pr, config, ngDevConfig, null, prTarget, git);
       expect(results.length).toBe(0);
     });
+
+    it('should allow merging when primitives changes have been merged already and PR has primitives and fw code', async () => {
+      const config = createIsolatedValidationConfig({assertIsolatedSeparateFiles: true});
+      let pr = createTestPullRequest();
+      const files = ['packages/core/primitives/rando.ts', 'packages/router/blah.ts'];
+      const fileHelper = PullRequestFiles.create(git, pr.number, googleSyncConfig);
+      const diffStats = {insertions: 0, deletions: 0, files: 1, separateFiles: 1, commits: 3};
+      spyOn(PullRequestFiles, 'create').and.returnValue(fileHelper);
+      spyOn(G3Stats, 'getDiffStats').and.returnValue(diffStats);
+      spyOn(fileHelper, 'loadPullRequestFiles').and.returnValue(Promise.resolve(files));
+      const results = await assertValidPullRequest(pr, config, ngDevConfig, null, prTarget, git);
+      expect(results.length).toBe(0);
+    });
   });
 });
 
diff --git a/ng-dev/pr/common/validation/assert-isolated-separate-files.ts b/ng-dev/pr/common/validation/assert-isolated-separate-files.ts
index ed0d58766..394948bd5 100644
--- a/ng-dev/pr/common/validation/assert-isolated-separate-files.ts
+++ b/ng-dev/pr/common/validation/assert-isolated-separate-files.ts
@@ -46,6 +46,8 @@ class Validation extends PullRequestValidation {
     if (g3SyncConfigWithMatchers === null) {
       return;
     }
+
+    // diffStats tells you what's already been merged in github, but hasn't yet been synced to G3
     const diffStats = await getDiffStats(config, g3SyncConfigWithMatchers.config, gitClient);
     if (diffStats === undefined) {
       return;
@@ -57,14 +59,24 @@ class Validation extends PullRequestValidation {
       g3SyncConfigWithMatchers.config,
     ).pullRequestHasSeparateFiles();
 
-    // if has merged stuff already and hasSeparateSyncFiles, it's safe to merge more separate sync files
+    // This validation applies to PRs that get merged when changes have not yet been synced into G3.
+    // The rules are as follows:
+    //   1. if pure framework changes have been merged, separate file changes should not be merged.
+    //   2. if separate file changes have been merged, pure framework changes should not be merged.
+    //   3. if separate file changes have been merged, any change merged MUST have separate file changes in it.
+    //   4. framework changes can be merged with separate file changes as long as that change ALSO
+    //       has separate file changes also.
+
+    // covers 2 & 3
     if (diffStats.separateFiles > 0 && !hasSeparateSyncFiles) {
       throw this._createError(
         `This PR cannot be merged as Shared Primitives code has already been merged. ` +
           `Primitives and Framework code must be merged and synced separately. Try again after a g3sync has finished.`,
       );
     }
-    if (diffStats.files > 0 && hasSeparateSyncFiles) {
+
+    // covers 1 & 4
+    if (diffStats.files > 0 && diffStats.separateFiles === 0 && hasSeparateSyncFiles) {
       throw this._createError(
         `This PR cannot be merged as Angular framework code has already been merged. ` +
           `Primitives and Framework code must be merged and synced separately. Try again after a g3sync has finished.`,