Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Fixes Divisions V5 apis to respond with RFC3339 date/time Format#7612

Merged
srijeet0406 merged 7 commits intoapache:masterfrom
jagan-parthiban:improve/rfc3339-divisions
Jul 13, 2023
Merged

Fixes Divisions V5 apis to respond with RFC3339 date/time Format#7612
srijeet0406 merged 7 commits intoapache:masterfrom
jagan-parthiban:improve/rfc3339-divisions

Conversation

@jagan-parthiban
Copy link
Copy Markdown
Contributor

@jagan-parthiban jagan-parthiban commented Jul 3, 2023

Closes: #7611
Related: #5911

  • Documentation
  • Traffic Ops

What is the best way to verify this PR?

Make Api calls to Divisions 5.0

Divisions to Use RFC3339 Format

curl --request GET \
  --url https://localhost:8443/api/5.0/divisions


{
	"response": [
		{
			"id": 33,
			"lastUpdated": "2023-05-19T09:39:09.761097+05:30",
			"name": "Chennai"
		},
		{
			"id": 34,
			"lastUpdated": "2023-06-26T15:29:31.872822+05:30",
			"name": "Mumbai"
		}
	]
}

If this is a bugfix, which Traffic Control versions contained the bug?

  • 7.0.1

PR submission checklist

@jagan-parthiban jagan-parthiban changed the title RFC3339 for Divisions V5 API Fixes Divisions V5 apis to respond with RFC3339 date/time Format Jul 3, 2023
@jagan-parthiban jagan-parthiban force-pushed the improve/rfc3339-divisions branch from 5512b58 to 675aa9d Compare July 3, 2023 04:20
@jagan-parthiban jagan-parthiban marked this pull request as ready for review July 3, 2023 11:32
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 3, 2023 •
edited
Loading

Codecov Report

Merging #7612 (de81ab9) into master (8a08e3b) will increase coverage by 5.53%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #7612      +/-   ##
============================================
+ Coverage     30.13%   35.67%   +5.53%     
  Complexity       98       98              
============================================
  Files           794      408     -386     
  Lines         84095    31320   -52775     
  Branches        907       90     -817     
============================================
- Hits          25344    11173   -14171     
+ Misses        56622    19073   -37549     
+ Partials       2129     1074    -1055
Flag Coverage Δ
golib_unit 48.29% <ø> (-0.01%) ⬇️
grove_unit 4.60% <ø> (ø)
t3c_unit ?
traffic_monitor_unit 21.30% <ø> (ø)
traffic_ops_integration 65.94% <ø> (-3.48%) ⬇️
traffic_ops_unit ?
traffic_portal_v2 ?
traffic_stats_unit 10.14% <ø> (ø)
unit_tests 31.36% <ø> (+4.09%) ⬆️
v3 57.79% <ø> (ø)
v4 79.18% <ø> (ø)
v5 ?

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

see 389 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

github-advanced-security[bot]
github-advanced-security AI found potential problems Jul 3, 2023
View reviewed changes
Comment thread traffic_ops/traffic_ops_golang/division/divisions.go

selectQuery := "SELECT id, name, last_updated FROM division d"
query := selectQuery + where + orderBy + pagination
rows, err := tx.NamedQuery(query, queryValues)

Check failure

Code scanning / CodeQL

Database query built from user-controlled sources

This query depends on a [user-provided value](1).
Copy link
Copy Markdown
Contributor

@kdamichie kdamichie Jul 10, 2023

Choose a reason for hiding this comment

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

Can you make the query assembly compliant with CodeQL?

@ocket8888 ocket8888 added Traffic Ops related to Traffic Ops low impact affects only a small portion of a CDN, and cannot itself break one tech debt rework due to choosing easy/limited solution labels Jul 3, 2023
@jagan-parthiban jagan-parthiban force-pushed the improve/rfc3339-divisions branch from 66ddbdc to de81ab9 Compare July 7, 2023 04:01
kdamichie
kdamichie approved these changes Jul 12, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@kdamichie kdamichie left a comment

Choose a reason for hiding this comment

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

Approved

Comment thread traffic_ops/traffic_ops_golang/division/divisions.go

selectQuery := "SELECT id, name, last_updated FROM division d"
query := selectQuery + where + orderBy + pagination
rows, err := tx.NamedQuery(query, queryValues)
Copy link
Copy Markdown
Contributor

@kdamichie kdamichie Jul 10, 2023

Choose a reason for hiding this comment

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

Can you make the query assembly compliant with CodeQL?

@ocket8888
Copy link
Copy Markdown
Contributor

ocket8888 commented Jul 12, 2023

Can you make the query assembly compliant with CodeQL?

No, CodeQL will complain every time we write a query unless/until we adopt an ORM.

@srijeet0406 srijeet0406 merged commit 068d30f into apache:master Jul 13, 2023
@rimashah25 rimashah25 added this to the 8.0.0 milestone Jul 17, 2023
@jagan-parthiban jagan-parthiban deleted the improve/rfc3339-divisions branch July 27, 2023 04:23
@asf-ci-trafficcontrol asf-ci-trafficcontrol mentioned this pull request Aug 1, 2023
Merged
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@kdamichie kdamichie kdamichie approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

low impact affects only a small portion of a CDN, and cannot itself break one tech debt rework due to choosing easy/limited solution Traffic Ops related to Traffic Ops

Projects

None yet

Milestone

8.0.0

Development

Successfully merging this pull request may close these issues.

Divisions in TO API uses non-RFC3339 date/time strings

6 participants

@jagan-parthiban @ocket8888 @kdamichie @github-advanced-security @rimashah25 @srijeet0406