Skip to content

Conversation

@ivan-ottinger
Copy link
Contributor

Related issues

Proposed Changes

  • filter out sites owned by a8c

Testing Instructions

  1. Check out the PR branch and build the app with npm start.
  2. Head over to the Sync tab of any local site and click on the Connect site button.
  3. Observe that the list of sites in the modal does not include any a8c sites.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

@ivan-ottinger ivan-ottinger self-assigned this Apr 23, 2025
@ivan-ottinger ivan-ottinger requested a review from a team April 23, 2025 14:27
Copy link
Contributor

@wojtekn wojtekn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The list of sites looks much cleaner now. 🥳

@ivan-ottinger ivan-ottinger changed the title Sync: Filter out a8c sites from the sync sites filtering Sync: Filter out a8c sites from the sync sites listing Apr 23, 2025
@ivan-ottinger ivan-ottinger merged commit 8ab546d into trunk Apr 24, 2025
9 checks passed
@ivan-ottinger ivan-ottinger deleted the update/sync-sites-filering-to-exclude-a8c-sites branch April 24, 2025 07:41
} );

const STUDIO_SYNC_FEATURE_NAME = 'studio-sync';
const A8C_SITE_OWNER_ID = 26957695;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think exposing this ID is a good approach.
We also do it in calypso, but I believe it would be safer if we could handle this in the API instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about the same as well, but since I have seen the ID used in public Calypso repo already, I followed the same approach. Let's continue the discussion in STU-390-linear-issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants