Skip to content

NOISSUE - Refactor listing for rules and reports#433

Merged
dborovcanin merged 10 commits intoabsmach:mainfrom
nyagamunene:fix_listing_for_rules_and_reports
Mar 16, 2026
Merged

NOISSUE - Refactor listing for rules and reports#433
dborovcanin merged 10 commits intoabsmach:mainfrom
nyagamunene:fix_listing_for_rules_and_reports

Conversation

@nyagamunene
Copy link
Contributor

What type of PR is this?

What does this do?

Which issue(s) does this PR fix/relate to?

  • Related Issue #
  • Resolves #

Have you included tests for your changes?

Did you document any new/modified feature?

Notes

@nyagamunene nyagamunene force-pushed the fix_listing_for_rules_and_reports branch from e4d78e3 to 8edaff4 Compare March 10, 2026 09:58
@nyagamunene nyagamunene marked this pull request as ready for review March 10, 2026 10:24
@codecov
Copy link

codecov bot commented Mar 10, 2026

Codecov Report

❌ Patch coverage is 48.83721% with 66 lines in your changes missing coverage. Please review.
✅ Project coverage is 22.98%. Comparing base (a031426) to head (3c24cd2).
⚠️ Report is 21 commits behind head on main.

Files with missing lines Patch % Lines
re/mocks/repository.go 0.00% 58 Missing ⚠️
re/postgres/repository.go 87.30% 4 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #433      +/-   ##
==========================================
- Coverage   25.29%   22.98%   -2.32%     
==========================================
  Files         142       25     -117     
  Lines       20549     4364   -16185     
==========================================
- Hits         5198     1003    -4195     
+ Misses      14913     3290   -11623     
+ Partials      438       71     -367     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nyagamunene nyagamunene force-pushed the fix_listing_for_rules_and_reports branch 2 times, most recently from 9c5cc43 to e34cbc4 Compare March 13, 2026 08:21
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@nyagamunene nyagamunene force-pushed the fix_listing_for_rules_and_reports branch from 7852c33 to 9616066 Compare March 13, 2026 13:47
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Copy link
Contributor

@dborovcanin dborovcanin left a comment

Choose a reason for hiding this comment

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

Add listing rules and reports service tests for SuperAdmin.

Comment on lines +429 to +454
pgData := ""
if pm.Limit != 0 {
pgData = "LIMIT :limit"
}
if pm.Offset != 0 {
pgData += " OFFSET :offset"
}
pq := pageRulesQuery(pm)

dir := api.DescDir
if pm.Dir == api.AscDir {
dir = api.AscDir
}

orderClause := ""

switch pm.Order {
case api.NameKey:
orderClause = fmt.Sprintf("ORDER BY name %s, id %s", dir, dir)
case api.CreatedAtOrder:
orderClause = fmt.Sprintf("ORDER BY created_at %s, id %s", dir, dir)
case api.UpdatedAtOrder:
orderClause = fmt.Sprintf("ORDER BY COALESCE(updated_at, created_at) %s, id %s", dir, dir)
default:
orderClause = fmt.Sprintf("ORDER BY COALESCE(updated_at, created_at) %s, id %s", dir, dir)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a lot of duplication from the previous method. Try to extract to shared helper.

Copy link
Contributor

Choose a reason for hiding this comment

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

The same goes for reports.

re/service.go Outdated
Comment on lines +178 to +190
switch session.SuperAdmin {
case true:
page, err := re.repo.ListAllRules(ctx, pm)
if err != nil {
return Page{}, errors.Wrap(svcerr.ErrViewEntity, err)
}
return page, nil
default:
page, err := re.repo.ListUserRules(ctx, session.UserID, pm)
if err != nil {
return Page{}, errors.Wrap(svcerr.ErrViewEntity, err)
}
return page, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Use simpler if. Same goes for reports.

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@dborovcanin dborovcanin merged commit 7fb5dd7 into absmach:main Mar 16, 2026
4 of 5 checks passed
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.

2 participants