Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sles subscriptions count #167

Merged
merged 5 commits into from
Mar 14, 2022
Merged

Conversation

nelsonkopliku
Copy link
Member

@nelsonkopliku nelsonkopliku commented Mar 14, 2022

Ports this

s.db.Table("sles_subscriptions").Where("id", SlesIdentifier).Count(&count)

to tronto in order to properly count sles subscriptions.

Thanks again photofinish 😄

Copy link
Contributor

@dottorblaster dottorblaster left a comment

Choose a reason for hiding this comment

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

:shipit:

@nelsonkopliku nelsonkopliku force-pushed the fix_sles_subscriptions_count branch from f3d1f48 to 39aeffb Compare March 14, 2022 10:48
@nelsonkopliku nelsonkopliku force-pushed the fix_sles_subscriptions_count branch from 39aeffb to 4710b4b Compare March 14, 2022 11:06
{:error, _} ->
conn
|> put_status(:internal_server_error)
|> json(%{error: "An error occurred in retrieving SLES subscription count."})
Copy link
Member

Choose a reason for hiding this comment

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

great

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks, but unnecessary 😄

subscription_projection(identifier: "sle-module-server-applications")
end)

assert 12 = Repo.all(SlesSubscriptionReadModel) |> length()
Copy link
Member

@fabriziosestito fabriziosestito Mar 14, 2022

Choose a reason for hiding this comment

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

Suggested change
assert 12 = Repo.all(SlesSubscriptionReadModel) |> length()
assert 12 = SlesSubscriptionReadModel |> Repo.all() |> length()

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, thanks.


case Repo.one(query) do
{:error, _} = error ->
error
Copy link
Member

Choose a reason for hiding this comment

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

Repo.one returns nil or the entity, this case will never match

Copy link
Member Author

Choose a reason for hiding this comment

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

yup 🙏

Copy link
Member

@fabriziosestito fabriziosestito left a comment

Choose a reason for hiding this comment

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

Couple of nitpicks and good to go

@nelsonkopliku nelsonkopliku force-pushed the fix_sles_subscriptions_count branch from f3cd26b to c507e9a Compare March 14, 2022 11:36
@fabriziosestito fabriziosestito merged commit 306e3d5 into master Mar 14, 2022
@fabriziosestito fabriziosestito deleted the fix_sles_subscriptions_count branch March 14, 2022 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants