Skip to content

Add routeEnum with validation and enhance error handling in BaseService [Twitter Gem TeamCity Packagist MozillaObservatory HexPm Greasyfork] - #11800

Merged
jNullj merged 26 commits into
badges:masterfrom
jNullj:core/route-enum-refactor-p1
Aug 1, 2026
Merged

Add routeEnum with validation and enhance error handling in BaseService [Twitter Gem TeamCity Packagist MozillaObservatory HexPm Greasyfork]#11800
jNullj merged 26 commits into
badges:masterfrom
jNullj:core/route-enum-refactor-p1

Conversation

@jNullj

@jNullj jNullj commented Apr 11, 2026

Copy link
Copy Markdown
Member

A step towards #11371 and express.js

some impotent points:

  1. We can't use enum in route anymore in future versions of dependencies, this is a refactor is part 1 to solve this
  2. i tried to get the most elegant and efficient solution at the cost of flexibility
  3. the code assumes and limits enum only the first param, i couldn't find any services with multiple enum params and non first param as enum, it also makes semse
  4. if any exception arise, we can always solve them at handle()
  5. i could make a multi param system but it would be more costly in run-time then a single include check in an array.
  6. this will require many follow up PR for all services...

@jNullj jNullj added the core Server, BaseService, GitHub auth, Shared helpers label Apr 11, 2026
@github-actions

github-actions Bot commented Apr 11, 2026

Copy link
Copy Markdown
Contributor
Warnings
⚠️ This PR modified service code for gem but not its test code.
That's okay so long as it's refactoring existing code.
⚠️ This PR modified service code for greasyfork but not its test code.
That's okay so long as it's refactoring existing code.
⚠️ This PR modified service code for hexpm but not its test code.
That's okay so long as it's refactoring existing code.
⚠️ This PR modified service code for mozilla-observatory but not its test code.
That's okay so long as it's refactoring existing code.
⚠️ This PR modified service code for packagist but not its test code.
That's okay so long as it's refactoring existing code.
⚠️ This PR modified service code for teamcity but not its test code.
That's okay so long as it's refactoring existing code.
⚠️ This PR modified service code for twitter but not its test code.
That's okay so long as it's refactoring existing code.
⚠️

Found badge URL that may not follow our standard route abbreviations in services/gem/gem-downloads.service.js.
Please ensure you've reviewed our conventions.

Messages
📖 ✨ Thanks for your contribution to Shields, @jNullj!
📖

Thanks for contributing to our documentation. We ❤️ our documentarians!

Generated by 🚫 dangerJS against 7f8aba5

pull Bot pushed a commit to mikeyhodl/shields that referenced this pull request May 5, 2026
…adges#11812)

part of issue badges#3329
helps unblock badges#11800

Co-authored-by: Copilot <copilot@github.com>
github-merge-queue Bot pushed a commit that referenced this pull request May 10, 2026
* refactor: replace route format with pattern for [travis]

part of issue #3329
helps unblock #11800

Co-authored-by: Copilot <copilot@github.com>

* fix: remove redundant '/com' prefix from Travis tests

* remove php-v route exclusion

---------

Co-authored-by: Copilot <copilot@github.com>
jNullj added a commit to jNullj/shields-fun-fork that referenced this pull request May 13, 2026
replace `route`'s deprecated `format` with `pattern`

part of badges#3329
helps badges#11800
pull Bot pushed a commit to victorv/shields that referenced this pull request May 14, 2026
…ce (badges#11858)

replace `route`'s deprecated `format` with `pattern`

part of badges#3329
helps badges#11800
jNullj added 2 commits July 11, 2026 23:10
firstParamName was used as litteral string insted of evaludating the value of the actual firstParamName extracted beforehand.
@jNullj
jNullj marked this pull request as ready for review July 12, 2026 19:45
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Updated review app: https://pr-11800-badges-shields.fly.dev

Comment thread doc/badge-urls.md Outdated
Comment thread services/test-helpers.js Outdated
Comment thread doc/TUTORIAL.md Outdated
- `pattern` defines the variable part of the route, everything that comes after `/example/`. It can include any number of named parameters. These are converted into regular expressions by [`path-to-regexp`][path-to-regexp]. Because a service instance won't be created until it's time to handle a request, the route and other metadata must be obtained by examining the classes themselves. [That's why they're marked `static`.][static]
- There is additional documentation on conventions for [designing badge URLs](./badge-urls.md)

- `routeEnum` (optional): an array of strings that defines a finite set of allowed values for an enum-like parameter in the route. When `routeEnum` is present, the first named parameter in the route `pattern` is treated as the enum value to validate against `routeEnum`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Indentation of these new elements is no consistent with that of other parameters:
Image

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

fixed at 3d56bc5

image

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@jNullj was it intentional to drop the link to ./badge-urls.md in this commit? It felt useful to me.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@PyvesB it looked like its getting a bit too convoluted with so many internal lines so i tried to reduce it a bit
i think it was nice to have but its one of the things i could cut out without hurting the main content.

i don't mind getting it back, let me see if i can make it a bit more clear this time

@jNullj

jNullj commented Aug 1, 2026

Copy link
Copy Markdown
Member Author

test failure: only gemrank failed, but it also fails in daily tests, we get timout and such, unrelated to this pr.

@jNullj
jNullj requested a review from PyvesB August 1, 2026 15:36
PyvesB
PyvesB previously approved these changes Aug 1, 2026
@jNullj

jNullj commented Aug 1, 2026

Copy link
Copy Markdown
Member Author

moved it back to where it was, i just realized i moved it into routeEnum by mistake.

@jNullj
jNullj requested a review from PyvesB August 1, 2026 20:05
@jNullj
jNullj enabled auto-merge August 1, 2026 20:20
@jNullj
jNullj disabled auto-merge August 1, 2026 20:20
@jNullj
jNullj merged commit d64eda3 into badges:master Aug 1, 2026
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Server, BaseService, GitHub auth, Shared helpers

Development

Successfully merging this pull request may close these issues.

2 participants