github-community-projects / private-mirrors

A GitHub App that allows you to contribute upstream using private mirrors of public projects
MIT License
119 stars 12 forks source link

fix: Enforce repository name length limit in dialogs #171

Closed zkoppert closed 2 months ago

zkoppert commented 2 months ago

Pull Request

Proposed Changes

fixes #135

This pull request limits the length of repository names in the application. The changes ensure that both the creation and editing of mirrors enforce a maximum length of 100 characters for the repository name. This is achieved through changes in the frontend and backend validation schemas.

For Reviewers

Readiness Checklist

Author/Contributor

Reviewer

ajhenry commented 2 months ago

Nice! You can add tests for the backend like in this example. Just pass your invalid value here

https://github.com/github-community-projects/internal-contribution-forks/blob/e5dcd387551976576228137a7357c01187ad5eb2/test/server/repos.test.ts#L245-L256

You can use this test case to base it on

zkoppert commented 2 months ago

✅ Added a test for going over the limit cc79061 👎🏻 Not happy with how the error.message looks in the expect statement. It's a multiline string. Open to any ideas/suggestions/improvements!

ajhenry commented 2 months ago

@zkoppert I think it’s good as is but you can always do a .includes("String must contain at most 100 character(s)") on the error message

zkoppert commented 2 months ago

✅ More readable test assertion

zkoppert commented 2 months ago

I checked out the contributing doc and didn't see anything about deploying the change. Should I deploy before merge/after merge/not at all?

sutterj commented 2 months ago

We've been deploying after merges. The deployment is from the internal repo which is (I think) why it's not mentioned here.