Skip to content

feat(api): Geo related field addition in the feed_response. Closes #524#909

Merged
regulartim merged 2 commits intoGreedyBear-Project:developfrom
drona-gyawali:api/geoip
Mar 2, 2026
Merged

feat(api): Geo related field addition in the feed_response. Closes #524#909
regulartim merged 2 commits intoGreedyBear-Project:developfrom
drona-gyawali:api/geoip

Conversation

@drona-gyawali
Copy link
Copy Markdown
Contributor

@drona-gyawali drona-gyawali commented Feb 28, 2026 •
edited
Loading

Description

Initially, I tried a simpler approach: adding an ArrayAgg annotation for sensor_countries in get_queryset and including it in a feed_response inside the base_fields. This was straightforward and clean just two lines of change.

Issue with initial approach:

  • Sensor_countries comes from a ManyToMany relation (IOC.sensors → Sensor.country). Annotating it in get_queryset (ArrayAgg) works only for that specific feed view.Any other place using the same queryset (scoring, cronjobs, tests) doesn’t have that annotation. Also When feeds_response calls .values(*required_fields) with "sensor_countries" in base_fields, Django tries to look it up as a real model field, which fails causing FieldError.

Current Approach:

  • Removed sensor_countries from base_fields to prevent errors in non-feed code.

  • Implemented get_sensor_countries_map to fetch sensor countries in batch, avoiding N+1 queries.

  • The feed response now includes sorted sensor_countries per IOC, along with attacker_country and other base fields.

Related issues

closes: #524
related PR : #880

Type of change

  • Bug fix (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).
  • Chore (refactoring, dependency updates, CI/CD changes, code cleanup, docs-only changes).

Checklist

Please complete this checklist carefully. It helps guide your contribution and lets maintainers verify that all requirements are met.

Formalities

  • I have read and understood the rules about how to Contribute to this project.
  • I chose an appropriate title for the pull request in the form: <feature name>. Closes #999
  • My branch is based on develop.
  • The pull request is for the branch develop.
  • I have reviewed and verified any LLM-generated code included in this PR.

Docs and tests

  • I documented my code changes with docstrings and/or comments.
  • I have checked if my changes affect user-facing behavior that is described in the docs. If so, I also created a pull request in the docs repository.
  • Linter (Ruff) gave 0 errors. If you have correctly installed pre-commit, it does these checks and adjustments on your behalf.
  • I have added tests for the feature/bug I solved.
  • All the tests gave 0 errors.

GUI changes

Ignore this section if you did not make any changes to the GUI.

  • I have provided a screenshot of the result in the PR.
  • I have created new frontend tests for the new component or updated existing ones.

Review process

  • We encourage you to create a draft PR first, even when your changes are incomplete. This way you refine your code while we can track your progress and actively review and help.
  • If you think your draft PR is ready to be reviewed by the maintainers, click the corresponding button. Your draft PR will become a real PR.
  • If your changes decrease the overall tests coverage (you will know after the Codecov CI job is done), you should add the required tests to fix the problem.
  • Every time you make changes to the PR and you think the work is done, you should explicitly ask for a review. After receiving a "change request", address the feedback and click "request re-review" next to the reviewer's profile picture at the top right.

@drona-gyawali
version(1): added geo field in api response
c86127a 
Signed-off-by: Dorna Raj Gyawali <dronarajgyawali@gmail.com>
@drona-gyawali drona-gyawali marked this pull request as ready for review February 28, 2026 17:48
regulartim
regulartim requested changes Mar 2, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@regulartim regulartim left a comment

Choose a reason for hiding this comment

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

Hey @drona-gyawali ! Sorry if I was not precise enough here: I don't think it makes a lot of sense to include the senor country in the responses. The attacker country is sufficient I guess. What do you think?

@regulartim
Copy link
Copy Markdown
Member

regulartim commented Mar 2, 2026

I have checked if my changes affect user-facing behavior that is described in the docs. If so, I also created a pull request in the docs repository.

I think your changes DOES affect user-facing behavior as you are adding fields to the API response.

@drona-gyawali
Copy link
Copy Markdown
Contributor Author

drona-gyawali commented Mar 2, 2026

The attacker country is sufficient I guess. What do you think?

Thanks for the review I added the sensor countries as an experiment. My initial thought was: we can already see the attacker country, but what if we also want to know where the IOC was actually detected? For example, if the attacker country is Nepal, the attack might have been observed from Germany this could give richer context for analysis.

That said, I’m completely fine reverting this change and keeping only the attacker country in the response.

Once again, thank you for your guidance!

@regulartim
Copy link
Copy Markdown
Member

regulartim commented Mar 2, 2026

Thanks for the review I added the sensor countries as an experiment. My initial thought was: we can already see the attacker country, but what if we also want to know where the IOC was actually detected? For example, if the attacker country is Nepal, the attack might have been observed from Germany this could give richer context for analysis.

Yeah, I see what you mean. I'll keep this in the back of my head. The Sensor model does not contain much information right now, but that will likely change in the near future. Then, we need to decide if and how we integrate sensor information into existing endpoint. I would like to postpone your idea to that day.

Once again, thank you for your guidance!

Thank you for your work! :)

@drona-gyawali drona-gyawali mentioned this pull request Mar 2, 2026
Merged
4 tasks
@drona-gyawali drona-gyawali requested a review from regulartim March 2, 2026 13:45
regulartim
regulartim approved these changes Mar 2, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@regulartim regulartim left a comment

Choose a reason for hiding this comment

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

Looks good! :)

@regulartim regulartim merged commit cc55456 into GreedyBear-Project:develop Mar 2, 2026
4 checks passed
@regulartim regulartim mentioned this pull request Mar 9, 2026
Merged
16 tasks
cclts pushed a commit to cclts/GreedyBear that referenced this pull request Mar 11, 2026
@drona-gyawali @cclts
feat(api): Geo related field addition in the feed_response. Closes Gr…
32e84d9 
…eedyBear-Project#524 (GreedyBear-Project#909)

* version(1): added geo field in api response

Signed-off-by: Dorna Raj Gyawali <dronarajgyawali@gmail.com>

* version(2): added only attacker_country

---------

Signed-off-by: Dorna Raj Gyawali <dronarajgyawali@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@regulartim regulartim regulartim approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@drona-gyawali @regulartim