Skip to content

Conversation

@samoehlert
Copy link
Collaborator

@samoehlert samoehlert commented Nov 19, 2025

Things we still need to do

  • Clean up resulting output on the API
  • Double check permissions
  • Write tests where we add an IP then hit the url to see if it's blocked or not

@samoehlert samoehlert added the enhancement New feature or request label Nov 19, 2025
@samoehlert samoehlert marked this pull request as draft November 19, 2025 06:03
@github-actions
Copy link

github-actions bot commented Nov 19, 2025

File Coverage
All files 82%
config/consumers.py 78%
config/urls.py 69%
config/settings/base.py 69%
config/settings/local.py 72%
scram/route_manager/admin.py 71%
scram/route_manager/models.py 70%
scram/route_manager/views.py 88%
scram/route_manager/api/serializers.py 92%
scram/route_manager/api/views.py 76%
scram/shared/shared_code.py 56%
scram/templates/403.html 91%
scram/templates/404.html 91%
scram/templates/base.html 99%

Minimum allowed coverage is 50%

Generated by 🐒 cobertura-action against 011a99f

@samoehlert
Copy link
Collaborator Author

I didn't change anything with behave tests and it seems to be failing instead of ignoring failures on 3.13 like the config says. Not sure if you feel the need to make me figure that out before merging this.

  behave_next_python:
    name: Run Behave
    runs-on: ubuntu-latest
    continue-on-error: true
    strategy:
      max-parallel: 4
      matrix:
        python-version: ['3.13']```

@samoehlert samoehlert marked this pull request as ready for review November 20, 2025 21:00
Copy link
Collaborator

@crankynetman crankynetman left a comment

Choose a reason for hiding this comment

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

Added a few comments inline. This works great for the exact use-case, but there are some edge cases that might be a bit confusing that I called out inline. I think we should fix the 500 due to submitting a subnet though just with a simple validation in the ViewSet itself if nothing else.

…r is missing

it was always returning true before which is bad UX at the least
…w which IP is active when searching a larger subnet
… search inside of them 2) add an informational warning if we had to no

rmalize the IP as a UX thing 3) make sure to include the actual IP with the response especially important if
 we return a lit
Copy link
Collaborator

@crankynetman crankynetman left a comment

Choose a reason for hiding this comment

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

Looks great, I think we just have one more kink to figure out around cidrs. And the answer might be as simple as document it and add it later, but it does seem like an ugly rake to step on later.

Copy link
Collaborator

@crankynetman crankynetman left a comment

Choose a reason for hiding this comment

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

Awesome work, nicely done!

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants