Follow-up from "Resolve "API endpoint to list the Docker images/tags of a group""
-
@ayufan started a discussion: Is there a reason why we pass
id:
andcontainer_type:
instead of passingsubject
and actual object?I think it should not be advised to perform manually a
.find(
.Especially, this is not really
container_type:
. It is ratherid_type:
, because it implies what is the meaning ofid:
.I would also say that this is advised from security point of view to also pass
current_user
and perform additional policy check that given user does have permission to access this operation. -
@ayufan started a discussion: This is also quite bad approach, as we do not sanity check the
container_type
, this code will fail if we pass some random ID, and container_type: :unknown.If we want to follow, but I see not reason to, we should prefer to use
case-when
.case container_type when :project project.container_repositories when :group group.container_repositories else raise ArgumentError, "invalid container_type" end
-
@ayufan started a discussion: It does not take repositories from nested groups.
-
@ayufan started a discussion: We should also validate if container registry is enabled for the project.
We seem to skip that when looking at given project, but also when getting a container repositories for groups.
-
@ayufan started a discussion: Is there a reason why we don't use
set(:reporter)
andset(:guest)
?I think also for a readability purposes it would be advised to add empty line in the future between blocks that use
set
andlet
.