feat: Filter flags by flags for pathContents #128

adrian-codecov · 2023-09-12T01:15:32Z

Purpose/Motivation

The client needs to be able to filter out files for pathcontents. This PR enables that by exposing filters to the client and the ability to reflect those files accordingly.

What does this PR do?

Meat of the PR

Adds logic in service/reports.py to calculate sessions based on flags, and files belonging to specific sessions
Adds logic to interface those changes to path contents
Adds the ability to filter files by flags for path contents related changes
Handles errors when selected flag leads to no result
Adds tests

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

…gic-to-filter-files-by-flag

codecov · 2023-09-12T01:24:06Z

Codecov Report

Merging #128 (7bb9624) into main (4028529) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 7bb9624 differs from pull request most recent head 84e7394. Consider uploading reports for the commit 84e7394 to get more accurate results

@@          Coverage Diff          @@
##            main    #128   +/-   ##
=====================================
  Coverage   95.30   95.30           
=====================================
  Files        702     702           
  Lines      14900   14930   +30     
=====================================
+ Hits       14199   14229   +30     
  Misses       701     701

Flag	Coverage Δ
unit	`95.49% <100.00%> (+<0.01%)`	⬆️
unit-latest-uploader	`95.49% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
graphql_api/types/commit/commit.py	`100.00% <100.00%> (ø)`
graphql_api/types/errors/errors.py	`100.00% <100.00%> (ø)`
graphql_api/types/path_contents/path_content.py	`100.00% <100.00%> (ø)`
services/path.py	`100.00% <100.00%> (ø)`
services/report.py	`96.03% <100.00%> (+0.43%)`	⬆️

adrian-codecov · 2023-09-12T02:07:08Z

services/path.py

+        files = report_service.files_belonging_to_flags(
+            commit_report=self.report, flags=self.filter_flags
+        )
+        self._filter_commit_report()


Not sure what I think about this, thought?

I would maybe just do this in the constructor so we don't have to rely on this method being called.

self.report = report if self.flags_filter: self.report = self.report.filter(flags=self.flags_filter)

I considered this, but the issue is that all the logic in the files_accounting_flags relies on a Report and not a FilteredReport. I suppose what I need to do is change that logic to use internally self.report(which is a FitleredReport by that point).report_file instead. I'll play around that

Oh yeah, good point. It's kinda strange to me that Report and FilteredReport don't offer the exact same API.

scott-codecov

Looks good! Just a few minor naming things you can feel free to ignore if you want

scott-codecov · 2023-09-12T17:23:43Z

graphql_api/types/inputs/path_contents_filters.graphql

@@ -2,9 +2,10 @@ input PathContentsFilters {
  searchValue: String
  displayType: PathContentDisplayType
  ordering: PathContentsOrdering
+  flags: [String]


Maybe make this [String!] so someone can't pass [None]

scott-codecov · 2023-09-12T17:25:13Z

services/path.py

+        files = report_service.files_belonging_to_flags(
+            commit_report=self.report, flags=self.filter_flags
+        )
+        self._filter_commit_report()


I would maybe just do this in the constructor so we don't have to rely on this method being called.

self.report = report if self.flags_filter: self.report = self.report.filter(flags=self.flags_filter)

scott-codecov · 2023-09-12T17:26:27Z

services/path.py

@@ -165,6 +171,19 @@ def __init__(
                if search_term.lower() in path.relative_path.lower()
            ]

+    @cached_property
+    def files_accounting_flags(self) -> List[str]:


I think you could just call this files (which takes into account it's own state of flag_filters). Outside callers don't necessarily care about the flag part (other than passing them into the constructor).

scott-codecov · 2023-09-12T17:28:45Z

services/report.py

+    return files_in_specific_sessions
+
+
+def calculate_sessions_with_specific_flags(


Not a huge fan of including things like calculate or compute in function names since that's basically the function of a computer. Maybe just sessions_with_flags?

Yeah that's fine, I remember seeing somewhere this as a "good naming convention" as this is describing a verb instead of a noun, where nouns should be more like @propertys (which I can change to too)

Hmm, interesting. I'd say if it's mutating state somewhere then a verb definitely makes sense. If it's just returning data then maybe less important to make that distinction?

Yeaaah makes sense to me, I can adjust this accordingly 👌

* main: (58 commits) Adding beginnings of GHA CI (#127) feat: Filter flags by flags for pathContents (#128) Create checkbox in Owner form in Django admin to set uses_invoice (#109) build(deps): bump certifi from 2020.6.20 to 2023.7.22 (#32) Feature/no compile (#126) Bump django from 4.2.2 to 4.2.3 (#42) Don't compile since source is available (#106) feat: Add firstPull resolver to GraphQL pull type (#108) chore: Upgrade requests and redis dependencies (#124) Update LICENSE (#122) Attempt migration (#121) 359 adjust monthly uploads for trialled customers (#119) Add changes for monthly uploads to account for trialing customer (#101) Adjust donwload_url link (#115) update to handle to redirects (#113) fix: Include impacted files with no coverage diff and no indirect changes in direct changes list (#114) Make uses_invoice field on Owner(#92) feat: support gh refresh tokens (#85) Add RiskyAlterField to utils/migrations (#93) Fix/config error enterprise (#107) ...

* main: (74 commits) Fix indentation error (#133) Add cache cleanup (#130) Feature/lint pre commit (#134) feat: trigger label analysis task after update (#131) Filter file comparisons by flags (#129) chore: Remove hard-coded dev BB redirect URL (#132) feat: Validate OIDC ID token during Sentry OAuth flow (#52) Adding beginnings of GHA CI (#127) feat: Filter flags by flags for pathContents (#128) Create checkbox in Owner form in Django admin to set uses_invoice (#109) build(deps): bump certifi from 2020.6.20 to 2023.7.22 (#32) Feature/no compile (#126) Bump django from 4.2.2 to 4.2.3 (#42) Don't compile since source is available (#106) feat: Add firstPull resolver to GraphQL pull type (#108) chore: Upgrade requests and redis dependencies (#124) Update LICENSE (#122) Attempt migration (#121) 359 adjust monthly uploads for trialled customers (#119) Add changes for monthly uploads to account for trialing customer (#101) ...

adrian-codecov added 3 commits September 11, 2023 18:49

Add logic to filter files by flags

9e3220c

Merge branch 'main' of github.com:codecov/codecov-api into 451-add-lo…

135a037

…gic-to-filter-files-by-flag

Added some tests

ab260f6

adrian-codecov commented Sep 12, 2023

View reviewed changes

another test

7bb9624

scott-codecov approved these changes Sep 12, 2023

View reviewed changes

add pr suggestions

84e7394

adrian-codecov merged commit eae5704 into main Sep 12, 2023
8 checks passed

adrian-codecov deleted the 451-add-logic-to-filter-files-by-flag branch September 12, 2023 21:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Filter flags by flags for pathContents #128

feat: Filter flags by flags for pathContents #128

adrian-codecov commented Sep 12, 2023

codecov bot commented Sep 12, 2023 •

edited

adrian-codecov Sep 12, 2023

scott-codecov Sep 12, 2023

adrian-codecov Sep 12, 2023

scott-codecov Sep 12, 2023

scott-codecov left a comment

scott-codecov Sep 12, 2023

scott-codecov Sep 12, 2023

scott-codecov Sep 12, 2023 •

edited

scott-codecov Sep 12, 2023

adrian-codecov Sep 12, 2023

scott-codecov Sep 12, 2023

adrian-codecov Sep 12, 2023

		return files_in_specific_sessions


		def calculate_sessions_with_specific_flags(

feat: Filter flags by flags for pathContents #128

feat: Filter flags by flags for pathContents #128

Conversation

adrian-codecov commented Sep 12, 2023

Purpose/Motivation

What does this PR do?

Meat of the PR

Legal Boilerplate

codecov bot commented Sep 12, 2023 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scott-codecov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scott-codecov Sep 12, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Sep 12, 2023 •

edited

scott-codecov Sep 12, 2023 •

edited