Skip to content
This repository has been archived by the owner on Oct 17, 2020. It is now read-only.

Add delete option for change log in repo #835

Merged
merged 4 commits into from
Jun 4, 2020
Merged

Conversation

rohithbalaji123
Copy link
Member

Part of #612

Description

Add delete APIs in ChangeLog repository and changeLog sqlDB adaptor.

@rohithbalaji123 rohithbalaji123 added the enhancement New feature or request label Jun 4, 2020
@rohithbalaji123 rohithbalaji123 self-assigned this Jun 4, 2020
@rohithbalaji123 rohithbalaji123 changed the title Add delete option for change log repo Add delete option for change log in repo Jun 4, 2020
@kapillamba4
Copy link
Member

kapillamba4 commented Jun 4, 2020

@rohithbalaji123 what will the graphQL schema look like for deleteChangeLog mutation ? PR looks good otherwise. Does the resolver return a boolean?

@rohithbalaji123
Copy link
Member Author

@kapillamba4 I was thinking of making it return the ID again if successful or nil when failed.

type AuthMutation {
	deleteChange(ID: String!): String
}

I was a bit inclined towards ID/nil instead of true/false because, it gives a bit more context while serving the same purpose. What do you think?

@kapillamba4
Copy link
Member

kapillamba4 commented Jun 4, 2020

@rohithbalaji123, i agree returning ID/nil seems better option

@codecov
Copy link

codecov bot commented Jun 4, 2020

Codecov Report

Merging #835 into master will increase coverage by 9.87%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #835      +/-   ##
==========================================
+ Coverage   40.19%   50.07%   +9.87%     
==========================================
  Files         111      123      +12     
  Lines        2035     2668     +633     
  Branches      149      149              
==========================================
+ Hits          818     1336     +518     
- Misses       1185     1282      +97     
- Partials       32       50      +18     
Flag Coverage Δ
#golang 70.70% <100.00%> (+8.10%) ⬆️
#typescript 23.49% <ø> (ø)
Impacted Files Coverage Δ
backend/app/adapter/sqldb/changelog.go 89.28% <100.00%> (ø)
backend/app/adapter/sqldb/bool.go 100.00% <0.00%> (ø)
backend/app/adapter/sqldb/user_changelog.go 92.85% <0.00%> (ø)
backend/app/adapter/sqldb/facebooksso.go 56.86% <0.00%> (ø)
backend/app/adapter/sqldb/featuretoggle.go 100.00% <0.00%> (ø)
backend/app/adapter/sqldb/short_link.go 91.42% <0.00%> (ø)
backend/app/adapter/sqldb/user_short_link.go 53.12% <0.00%> (ø)
backend/app/adapter/sqldb/googlesso.go 56.86% <0.00%> (ø)
backend/app/adapter/sqldb/time.go 100.00% <0.00%> (ø)
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9184c4e...04411f4. Read the comment docs.

backend/app/adapter/sqldb/changelog.go Outdated Show resolved Hide resolved
backend/app/usecase/repository/change_log_fake.go Outdated Show resolved Hide resolved
backend/app/usecase/repository/change_log_fake.go Outdated Show resolved Hide resolved
@magicoder10
Copy link
Member

Hi @rohithbalaji123 , thank you so much for working on this feature! Left few comments on the PR.

@magicoder10 magicoder10 merged commit 9342bc0 into master Jun 4, 2020
@magicoder10 magicoder10 deleted the change-delete-adapter branch June 4, 2020 17:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants