Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

5823 – Prevent an attempted relationship between items across teams #2149

Closed

Conversation

vasconsaurus
Copy link
Contributor

@vasconsaurus vasconsaurus commented Dec 4, 2024

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context – why has this been changed/fixed.

References: CV2-5676, CV2-5823

How has this been tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can verify the changes. Please describe whether or not you implemented automated tests.

Things to pay attention to during code review

Please describe parts of the change that require extra attention during code review, for example:

  • File FFFF, line LL: This refactoring does this and this. Is it consistent with how it’s implemented elsewhere?
  • Etc.

Checklist

  • I have performed a self-review of my own code
  • I have added unit and feature tests, if the PR implements a new feature or otherwise would benefit from additional testing
  • I have added regression tests, if the PR fixes a bug
  • I have added logging, exception reporting, and custom tracing with any additional information required for debugging
  • I considered secure coding practices when writing this code. Any security concerns are noted above.
  • I have commented my code in hard-to-understand areas, if any
  • I have made needed changes to the README
  • My changes generate no new warnings
  • If I added a third party module, I included a rationale for doing so and followed our current guidelines

@vasconsaurus vasconsaurus force-pushed the 5676-alegre-bot-must-filter-invalid-results branch from a167c7a to 46534fb Compare December 4, 2024 20:08
@vasconsaurus vasconsaurus force-pushed the 5676-alegre-bot-must-filter-invalid-results branch from 86c7659 to dba3e81 Compare December 10, 2024 13:58
def isolate_relevant_context(project_media, result)
(result["contexts"]||result["context"]).select{|x| ([x["team_id"]].flatten & [project_media.team_id].flatten).count > 0 && !x["temporary_media"]}.first
def isolate_relevant_context(team_id, result)
(result["contexts"]||result["context"]).select{|x| ([x["team_id"]].flatten & [team_id].flatten).count > 0 && !x["temporary_media"]}.first
Copy link
Contributor

Choose a reason for hiding this comment

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

probably needs to be [(result["contexts"]||result["context"])].flatten.select.... instead here

@vasconsaurus vasconsaurus changed the title 5676 – Alegre Bot must filter invalid results 5823 – Prevent an attempted relationship between items across teams Dec 11, 2024
response = Bot::Alegre.query_sync_with_params(params, "text")
results = response['result'].to_a.sort_by{ |result| result['_score'] }
response = Bot::Alegre.query_sync_with_params(params, "text")["result"].collect{|result|
result["context"] = Bot::Alegre.isolate_relevant_context(team_id, result)
Copy link
Contributor

Choose a reason for hiding this comment

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

For reasons I don't understand, Explainers appear to be indexed in Alegre with the team slug rather than the team_id. As a result, this won't work.

I think the correction action is to index explainers with team_id, but whatever the solution we need to work that out first.

@vasconsaurus vasconsaurus deleted the 5676-alegre-bot-must-filter-invalid-results branch December 16, 2024 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants