Skip to content

Prohibit identical class names within site PART 2#795

Open
rolivares93 wants to merge 8 commits intomainfrom
bug-737/prohibit-identical-class-names-within-site-2
Open

Prohibit identical class names within site PART 2#795
rolivares93 wants to merge 8 commits intomainfrom
bug-737/prohibit-identical-class-names-within-site-2

Conversation

@rolivares93
Copy link

Proposed changes

I refactored the useOrgNameExistsQuery.

Screen.Recording.2026-02-03.at.12.38.56.mov

Types of changes

What types of changes does this pull request introduce?

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that does not add functionality but makes code cleaner or more efficient)
  • Tests (new or updated tests)
  • Styles (changes to code styling)
  • CI (continuous integration changes)
  • Other (please describe below)

Additional Notes

@rolivares93 rolivares93 self-assigned this Feb 3, 2026
@rolivares93 rolivares93 linked an issue Feb 3, 2026 that may be closed by this pull request
@github-actions
Copy link

github-actions bot commented Feb 3, 2026

Visit the preview URL for this PR (updated for commit c9dabf0):

https://hs-levante-admin-dev--pr795-bug-737-prohibit-ide-r5575q2b.web.app

(expires Mon, 23 Feb 2026 15:16:47 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 7889bff1da3bcc333d7422b9fc863c65b3962be7

@CCuskley
Copy link

CCuskley commented Feb 3, 2026

Testing works as expected - ready to merge pending code review.

Copy link
Member

@fhconte fhconte left a comment

Choose a reason for hiding this comment

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

Approved overall, with two important comments to consider before merging. Please review those points before merge.

},
};

try {
Copy link
Member

Choose a reason for hiding this comment

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

Non‑blocking suggestion: consider failing closed on query errors. Returning false on a network/index failure can allow creation and let duplicates slip through; maybe it may be safer to surface the error here.

return false;
}

const requestBody = {
Copy link
Member

Choose a reason for hiding this comment

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

When currentSite is 'any', the site‑scoped filter won’t match and duplicates can slip through. If this is the intended behavior, and maybe super admins are not even touching this part, all good.


try {
const response = await axios.post(`${documentPath}:runQuery`, requestBody);
const mappedData = mapFields(response?.data);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of using POST can't you use the query and getDocs exposed from firebase/firestore?
An example would be:

const assignmentsQuery = query(
  collection(db, "assignments"),
  orderBy("createdAt"),
  limit(PAGE_SIZE), // for pagination (optional)
  startAfter(lastDoc) // optional (for offset)
)

const snapshot = await getDocs(assignmentsQuery)

Potentially this can be moved to firekit as well.

Copy link
Author

Choose a reason for hiding this comment

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

This will be fixed as soon as we normalize the db, so we'll be able to create/re-create queries as necessary.

@rolivares93
Copy link
Author

@digital-pro why did you set this PR as blocked?

@digital-pro
Copy link
Collaborator

digital-pro commented Feb 19, 2026 via email

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] PERMISSIONS Prohibit identical class names within site

5 participants