Skip to content

Conversation

@ecrupper
Copy link
Contributor

Part of the effort for go-vela/community#343

  • Adds new query parameter to the GetOrgRepos function: sort_by
  • Utilize SQL query to return list of all org repos sorted by their latest build's created timestamp in descending order

@ecrupper ecrupper added the feature Indicates a new feature label Mar 15, 2022
@ecrupper ecrupper self-assigned this Mar 15, 2022
@ecrupper ecrupper requested a review from a team as a code owner March 15, 2022 18:44
@codecov
Copy link

codecov bot commented Mar 15, 2022

Codecov Report

Merging #611 (f65393c) into master (29562a4) will increase coverage by 0.01%.
The diff coverage is 65.38%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #611      +/-   ##
==========================================
+ Coverage   55.06%   55.07%   +0.01%     
==========================================
  Files         194      194              
  Lines       15704    15728      +24     
==========================================
+ Hits         8647     8662      +15     
- Misses       6697     6704       +7     
- Partials      360      362       +2     
Impacted Files Coverage Δ
api/repo.go 0.00% <0.00%> (ø)
api/scm.go 0.00% <0.00%> (ø)
api/user.go 0.00% <0.00%> (ø)
database/postgres/repo_list.go 89.91% <73.91%> (-1.83%) ⬇️
database/sqlite/repo_list.go 74.78% <73.91%> (-0.22%) ⬇️

cognifloyd
cognifloyd previously approved these changes Apr 5, 2022
Copy link
Member

@cognifloyd cognifloyd left a comment

Choose a reason for hiding this comment

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

LGTM.

I'm not sure what you can do (if anything) about Github's "Code scanning" warning: "Database query built from user-controlled sources" since that's how it was before.

jbrockopp
jbrockopp previously approved these changes Apr 20, 2022
Copy link
Contributor

@jbrockopp jbrockopp left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@wass3r wass3r left a comment

Choose a reason for hiding this comment

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

super minor suggestion. otherwise, looks good

// name: sort_by
// description: How to sort the results
// type: string
// default: name
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we add enum here? it can really only be name or latest right?

see example here:

server/api/secret.go

Lines 43 to 46 in 61358d2

// enum:
// - org
// - repo
// - shared

@ecrupper ecrupper dismissed stale reviews from jbrockopp and cognifloyd via 0806b08 April 21, 2022 16:13
Copy link
Contributor

@jbrockopp jbrockopp left a comment

Choose a reason for hiding this comment

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

LGTM

@ecrupper ecrupper merged commit d8f169f into master Apr 21, 2022
@ecrupper ecrupper deleted the i343/last-updated-repos-query branch April 21, 2022 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Indicates a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants