Skip to content

Conversation

@VishakhaSainani-Josh
Copy link
Collaborator

Issue resolved #3

  1. Get all users list from db to query BigQuery for previous day contributions
  2. Fetch contributions from BigQuery
  3. Parse the fetched contribution data
  4. Check if the repository related to the contribution exists
    If it does not exist:
    Create a new entry in the repositories table
    Retrieve the newly created repository ID
  5. Get the user who did that contribution
  6. Get the score to be assigned for the particular contribution type
  7. Insert the contribution into the contributions table using the repository ID


repoFetched, err := s.repositoryService.GetRepoByRepoId(ctx, contribution.RepoID)
repositoryId := repoFetched.Id
if err != nil {
Copy link

Choose a reason for hiding this comment

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

The error must be checked before the assignment

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

okay, I can put it in else then

}
}

func (s *service) ProcessFetchedContributions(ctx context.Context) error {
Copy link

Choose a reason for hiding this comment

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

This method is very big please check if we can we refactor this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

okay

"github.com/joshsoftware/code-curiosity-2025/internal/pkg/apperrors"
)

type repositoryRepository struct {
Copy link

Choose a reason for hiding this comment

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

can we have a different name for the struct

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We discussed about it, this one works for now as the standard naming kept for db layer is repository and we have a repository table in our system as well. If needed then I can change the repository folder name to dbstore or something else. This will initiate changes in all other parts as well. Let me know if I should.


req.Header.Add("Authorization", s.appCfg.GithubPersonalAccessToken)

client := &http.Client{}
Copy link

Choose a reason for hiding this comment

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

suggesstion -> Pass this in the service struct so that we can reuse it rather than creating a new client everytime

Comment on lines 50 to 55
if err := contributions.Next(&contribution); err == iterator.Done {
break
} else if err != nil {
slog.Error("error iterating contribution rows", "error", err)
break
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

error is being not returned here, try to refactor this as:

		if err != nil {
			if err == iterator.Done {
				break
			}

			slog.Error("error iterating contribution rows", "error", err)
			return err
		}


var repositoryId int
repoFetched, err := s.repositoryService.GetRepoByRepoId(ctx, contribution.RepoID) //err no rows
if err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Explicitly check for apperrors.ErrRepoNotFound

Comment on lines 66 to 77
repo, err := s.repositoryService.FetchRepositoryDetails(ctx, contribution.RepoUrl)
if err != nil {
slog.Error("error fetching repository details")
return err
}

repositoryCreated, err := s.repositoryService.CreateRepository(ctx, contribution.RepoID, repo)
if err != nil {
slog.Error("error creating repository", "error", err)
return err
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

move the remove creation logic to another function, to reduce cognitive complexity

Comment on lines 107 to 110
var action string
if actionVal, ok := contributionPayload["action"]; ok {
action = actionVal.(string)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it ok if action is empty, do we need to handle it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

action value cannot be empty, according to the events used all those have action as a key in their payload.

var contributionType string
switch contribution.Type {
case "PullRequestEvent":
if action == "closed" && isMerged {
Copy link
Collaborator

Choose a reason for hiding this comment

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

create constants for strings


import "time"

type RepoOWner struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit- typo

Comment on lines +53 to +70
resp, err := s.httpClient.Do(req)
if err != nil {
slog.Error("error fetching user repositories details", "error", err)
return FetchRepositoryDetailsResponse{}, err
}

body, err := io.ReadAll(resp.Body)
if err != nil {
slog.Error("error freading body", "error", err)
return FetchRepositoryDetailsResponse{}, err
}

var repoDetails FetchRepositoryDetailsResponse
err = json.Unmarshal(body, &repoDetails)
if err != nil {
slog.Error("error unmarshalling fetch repository details body", "error", err)
return FetchRepositoryDetailsResponse{}, err
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Create a utils function to do network calls, eg: DoPost, DoGet and so on

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

okay. Also I want to move the FetchRepositoryDetails and other functions which are calls to github API's in a separate service named github. So there I can call the utility. Is that fine?

Comment on lines 33 to 34
YesterdayDate := time.Now().AddDate(0, 0, -1)
YesterdayYearMonthDay := YesterdayDate.Format("20060102")
Copy link
Collaborator

Choose a reason for hiding this comment

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

define variables close to where they are used

Comment on lines 48 to 77
fetchDailyContributionsQuery := fmt.Sprintf(`
SELECT
id,
type,
public,
actor.id AS actor_id,
actor.login AS actor_login,
actor.gravatar_id AS actor_gravatar_id,
actor.url AS actor_url,
actor.avatar_url AS actor_avatar_url,
repo.id AS repo_id,
repo.name AS repo_name,
repo.url AS repo_url,
payload,
created_at,
other
FROM
githubarchive.day.%s
WHERE
type IN (
'IssuesEvent',
'PullRequestEvent',
'PullRequestReviewEvent',
'IssueCommentEvent',
'PullRequestReviewCommentEvent'
)
AND (
actor.login IN (%s)
)
`, YesterdayYearMonthDay, githubUsernames)
Copy link
Collaborator

Choose a reason for hiding this comment

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

define the query in a constant or another file

Comment on lines +48 to +77
executer := cr.BaseRepository.initiateQueryExecuter(tx)

var contribution Contribution
err := executer.QueryRowContext(ctx, createContributionQuery,
contributionInfo.UserId,
contributionInfo.RepositoryId,
contributionInfo.ContributionScoreId,
contributionInfo.ContributionType,
contributionInfo.BalanceChange,
contributionInfo.ContributedAt,
).Scan(
&contribution.Id,
&contribution.UserId,
&contribution.RepositoryId,
&contribution.ContributionScoreId,
&contribution.ContributionType,
&contribution.BalanceChange,
&contribution.ContributedAt,
&contribution.CreatedAt,
&contribution.UpdatedAt,
)
if err != nil {
slog.Error("error occured while inserting contributions", "error", err)
return Contribution{}, apperrors.ErrContributionCreationFailed
}

return contribution, err
}

func (cr *contributionRepository) GetContributionScoreDetailsByContributionType(ctx context.Context, tx *sqlx.Tx, contributionType string) (ContributionScore, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to handle no row error explicitly in this function

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.

Fetch Users Contributions

5 participants