Skip to content

Fix some private contributions being hidden on the contribution calendar

Dustin Eckhardt requested to merge eggerd/gitlab:eggerd-master-patch-56208 into master

What does this MR do and why?

Fixes the issue of some private contributions being hidden on the contributions calendar (#24137 (closed)), even though the contributor opted-in to show them.

For this the feature access check for the current_user (aka "visitor") will be skipped, if the feature is enabled. Because otherwise private projects included in @contributed_project_ids get filtered out again at this point, if current_user does not have access.

# feature access check
authed_projects = Project.where(id: @contributed_project_ids)
  .with_feature_available_for_user(feature, current_user)
  .reorder(nil)
  .select(:id)

The reason only some private projects are affected by this are inconsistent feature access levels. This allows them to get through the feature access check, while private projects with the correct feature access levels of 10 will get filtered out. So without this inconsistency, the "Include private contributions on my profile" feature wouldn't work at all.

Screenshots

Live version Fix applied
calendar-bug calendar-fixed

How to set up and validate locally

Before applying the fix:

  1. Enable the "Include private contributions on my profile" feature in your profile settings
  2. Create a new private project and contribute something (e.g. initialize with readme)
  3. Check your profile while not logged-in or being logged-in as another user - the contribution will be shown in the calendar
  4. (Optional) Check the feature access levels of the project using the psql cli (replace PROJECT_ID accordingly): 
    select repository_access_level,merge_requests_access_level,issues_access_level from project_features where project_id = PROJECT_ID;
    All three will be at 20 (everyone)
  5. Open the project settings and change the project visibility to Public, then Private, and then click Save changes
  6. (Optional) Repeat step 4 to check the feature access levels of the project again - now all three will be at the correct level of 10 (project members)
  7. Repeat step 3 - the contribution will now be hidden from the calendar

After applying the fix:
Repeat step 3 from above and notice that the contribution is now shown again.

Database

The changes of this MR alter the queries executed by:

Event.reorder(nil)
  .select(t[:project_id], t[:target_type], t[:action], "date(created_at + #{date_interval}) AS date", 'count(id) as total_amount')
  .group(t[:project_id], t[:target_type], t[:action], "date(created_at + #{date_interval})")
  .where(conditions)
  .where("events.project_id in (#{authed_projects})")

But only if the contributor opted-in to show private contributions on the calendar. Otherwise, the same queries as before are executed. Therefore, this section only lists queries that are executed while the contributor is opted-in.

Before

Please note that each of the queries below would be executed once for every of the following feature access levels that get checked: repository_access_level, merge_requests_access_level, issues_access_level. Because they only differ in this point, each case below only shows one of those queries.

`current_user` = contributor 
SELECT
    "events"."project_id",
    "events"."target_type",
    "events"."action",
    date(created_at + INTERVAL '0 seconds') AS date,
    count(id) AS total_amount
FROM
    "events"
WHERE
    "events"."created_at" >= '2020-11-24 00:00:00'
    AND "events"."created_at" <= '2021-11-24 23:59:59.999999'
    AND "events"."author_id" = 1
    AND (events.project_id IN (
            SELECT
                "projects"."id"
            FROM
                "projects"
            LEFT JOIN project_features ON projects.id = project_features.project_id
        WHERE
            "projects"."id" IN (22, 19, 8, 7, 6, 5, 4, 2, 1)
            AND ("project_features"."repository_access_level" > 0
                OR "project_features"."repository_access_level" IS NULL)))
GROUP BY
    "events"."project_id",
    "events"."target_type",
    "events"."action",
    date(created_at + INTERVAL '0 seconds')

Plan: https://explain.depesz.com/s/vmr2

`current_user` = other user 
SELECT
    "events"."project_id",
    "events"."target_type",
    "events"."action",
    date(created_at + INTERVAL '0 seconds') AS date,
    count(id) AS total_amount
FROM
    "events"
WHERE
    "events"."created_at" >= '2020-11-24 00:00:00'
    AND "events"."created_at" <= '2021-11-24 23:59:59.999999'
    AND "events"."author_id" = 1
    AND (events.project_id IN (
            SELECT
                "projects"."id"
            FROM
                "projects"
            LEFT JOIN project_features ON projects.id = project_features.project_id
        WHERE
            "projects"."id" IN (22, 19, 8, 7, 6, 5, 4, 2, 1)
            AND ("project_features"."repository_access_level" IS NULL
                OR "project_features"."repository_access_level" IN (20, 30)
                OR ("project_features"."repository_access_level" = 10
                    AND EXISTS (
                        SELECT
                            1
                        FROM
                            "project_authorizations"
                        WHERE
                            "project_authorizations"."user_id" = 97
                            AND (project_authorizations.project_id = projects.id)
                            AND (project_authorizations.access_level >= 10))))))
GROUP BY
    "events"."project_id",
    "events"."target_type",
    "events"."action",
    date(created_at + INTERVAL '0 seconds')

Plan: https://explain.depesz.com/s/aL5J

`current_user` = guest 
SELECT
    "events"."project_id",
    "events"."target_type",
    "events"."action",
    date(created_at + INTERVAL '0 seconds') AS date,
    count(id) AS total_amount
FROM
    "events"
WHERE
    "events"."created_at" >= '2020-11-24 00:00:00'
    AND "events"."created_at" <= '2021-11-24 23:59:59.999999'
    AND "events"."author_id" = 1
    AND (events.project_id IN (
            SELECT
                "projects"."id"
            FROM
                "projects"
            LEFT JOIN project_features ON projects.id = project_features.project_id
        WHERE
            "projects"."id" IN (22, 19, 8, 7, 6, 5, 4, 2, 1)
            AND ("project_features"."repository_access_level" IN (20, 30)
                OR "project_features"."repository_access_level" IS NULL)))
GROUP BY
    "events"."project_id",
    "events"."target_type",
    "events"."action",
    date(created_at + INTERVAL '0 seconds')

Plan: https://explain.depesz.com/s/vsLW

After

The current_user no longer influences the executed query, if the contributor has opted-in.

SELECT
    "events"."project_id",
    "events"."target_type",
    "events"."action",
    date(created_at + INTERVAL '0 seconds') AS date,
    count(id) AS total_amount
FROM
    "events"
WHERE
    "events"."created_at" >= '2020-11-24 00:00:00'
    AND "events"."created_at" <= '2021-11-24 23:59:59.999999'
    AND "events"."author_id" = 1
    AND (events.project_id IN (22, 19, 8, 7, 6, 5, 4, 2, 1))
GROUP BY
    "events"."project_id",
    "events"."target_type",
    "events"."action",
    date(created_at + INTERVAL '0 seconds')

Plan: https://explain.depesz.com/s/eKTI

Fetching Projects

The list of projects used for all this gets generated by the ContributedProjectsFinder. As requested during the discussion, the related query is shown below.

Query while opted-in 
SELECT
    "projects"."id",
    "projects"."name",
    "projects"."path",
    "projects"."description",
    "projects"."created_at",
    "projects"."updated_at",
    "projects"."creator_id",
    "projects"."namespace_id",
    "projects"."last_activity_at",
    "projects"."import_url",
    "projects"."visibility_level",
    "projects"."archived",
    "projects"."avatar",
    "projects"."merge_requests_template",
    "projects"."star_count",
    "projects"."merge_requests_rebase_enabled",
    "projects"."import_type",
    "projects"."import_source",
    "projects"."approvals_before_merge",
    "projects"."reset_approvals_on_push",
    "projects"."merge_requests_ff_only_enabled",
    "projects"."issues_template",
    "projects"."mirror",
    "projects"."mirror_user_id",
    "projects"."shared_runners_enabled",
    "projects"."runners_token",
    "projects"."build_coverage_regex",
    "projects"."build_allow_git_fetch",
    "projects"."build_timeout",
    "projects"."mirror_trigger_builds",
    "projects"."pending_delete",
    "projects"."public_builds",
    "projects"."last_repository_check_failed",
    "projects"."last_repository_check_at",
    "projects"."only_allow_merge_if_pipeline_succeeds",
    "projects"."has_external_issue_tracker",
    "projects"."repository_storage",
    "projects"."repository_read_only",
    "projects"."request_access_enabled",
    "projects"."has_external_wiki",
    "projects"."ci_config_path",
    "projects"."lfs_enabled",
    "projects"."description_html",
    "projects"."only_allow_merge_if_all_discussions_are_resolved",
    "projects"."repository_size_limit",
    "projects"."printing_merge_request_link_enabled",
    "projects"."auto_cancel_pending_pipelines",
    "projects"."service_desk_enabled",
    "projects"."cached_markdown_version",
    "projects"."delete_error",
    "projects"."last_repository_updated_at",
    "projects"."disable_overriding_approvers_per_merge_request",
    "projects"."storage_version",
    "projects"."resolve_outdated_diff_discussions",
    "projects"."remote_mirror_available_overridden",
    "projects"."only_mirror_protected_branches",
    "projects"."pull_mirror_available_overridden",
    "projects"."jobs_cache_index",
    "projects"."external_authorization_classification_label",
    "projects"."mirror_overwrites_diverged_branches",
    "projects"."pages_https_only",
    "projects"."external_webhook_token",
    "projects"."packages_enabled",
    "projects"."merge_requests_author_approval",
    "projects"."pool_repository_id",
    "projects"."runners_token_encrypted",
    "projects"."bfg_object_map",
    "projects"."detected_repository_languages",
    "projects"."merge_requests_disable_committers_approval",
    "projects"."require_password_to_approve",
    "projects"."emails_disabled",
    "projects"."max_pages_size",
    "projects"."max_artifacts_size",
    "projects"."remove_source_branch_after_merge",
    "projects"."marked_for_deletion_at",
    "projects"."marked_for_deletion_by_user_id",
    "projects"."autoclose_referenced_issues",
    "projects"."suggestion_commit_message",
    "projects"."project_namespace_id"
FROM ((
        SELECT
            "projects"."id",
            "projects"."name",
            "projects"."path",
            "projects"."description",
            "projects"."created_at",
            "projects"."updated_at",
            "projects"."creator_id",
            "projects"."namespace_id",
            "projects"."last_activity_at",
            "projects"."import_url",
            "projects"."visibility_level",
            "projects"."archived",
            "projects"."avatar",
            "projects"."merge_requests_template",
            "projects"."star_count",
            "projects"."merge_requests_rebase_enabled",
            "projects"."import_type",
            "projects"."import_source",
            "projects"."approvals_before_merge",
            "projects"."reset_approvals_on_push",
            "projects"."merge_requests_ff_only_enabled",
            "projects"."issues_template",
            "projects"."mirror",
            "projects"."mirror_user_id",
            "projects"."shared_runners_enabled",
            "projects"."runners_token",
            "projects"."build_coverage_regex",
            "projects"."build_allow_git_fetch",
            "projects"."build_timeout",
            "projects"."mirror_trigger_builds",
            "projects"."pending_delete",
            "projects"."public_builds",
            "projects"."last_repository_check_failed",
            "projects"."last_repository_check_at",
            "projects"."only_allow_merge_if_pipeline_succeeds",
            "projects"."has_external_issue_tracker",
            "projects"."repository_storage",
            "projects"."repository_read_only",
            "projects"."request_access_enabled",
            "projects"."has_external_wiki",
            "projects"."ci_config_path",
            "projects"."lfs_enabled",
            "projects"."description_html",
            "projects"."only_allow_merge_if_all_discussions_are_resolved",
            "projects"."repository_size_limit",
            "projects"."printing_merge_request_link_enabled",
            "projects"."auto_cancel_pending_pipelines",
            "projects"."service_desk_enabled",
            "projects"."cached_markdown_version",
            "projects"."delete_error",
            "projects"."last_repository_updated_at",
            "projects"."disable_overriding_approvers_per_merge_request",
            "projects"."storage_version",
            "projects"."resolve_outdated_diff_discussions",
            "projects"."remote_mirror_available_overridden",
            "projects"."only_mirror_protected_branches",
            "projects"."pull_mirror_available_overridden",
            "projects"."jobs_cache_index",
            "projects"."external_authorization_classification_label",
            "projects"."mirror_overwrites_diverged_branches",
            "projects"."pages_https_only",
            "projects"."external_webhook_token",
            "projects"."packages_enabled",
            "projects"."merge_requests_author_approval",
            "projects"."pool_repository_id",
            "projects"."runners_token_encrypted",
            "projects"."bfg_object_map",
            "projects"."detected_repository_languages",
            "projects"."merge_requests_disable_committers_approval",
            "projects"."require_password_to_approve",
            "projects"."emails_disabled",
            "projects"."max_pages_size",
            "projects"."max_artifacts_size",
            "projects"."remove_source_branch_after_merge",
            "projects"."marked_for_deletion_at",
            "projects"."marked_for_deletion_by_user_id",
            "projects"."autoclose_referenced_issues",
            "projects"."suggestion_commit_message",
            "projects"."project_namespace_id"
        FROM
            "projects"
        WHERE
            "projects"."id" IN ( SELECT DISTINCT
                    "events"."project_id"
                FROM
                    "events"
                WHERE (action = 5
                    OR (target_type IN ('MergeRequest', 'Issue')
                        AND action IN (1, 3, 7))
                    OR (target_type = 'Note'
                        AND action = 6))
                AND "events"."author_id" = 1
                AND (created_at > '2020-11-26 17:14:51.439981'))
            AND "projects"."id" IN (
                SELECT
                    "projects"."id"
                FROM
                    "projects"
                    INNER JOIN "project_authorizations" ON "projects"."id" = "project_authorizations"."project_id"
                WHERE
                    "project_authorizations"."user_id" = 1))
        UNION (
            SELECT
                "projects"."id",
                "projects"."name",
                "projects"."path",
                "projects"."description",
                "projects"."created_at",
                "projects"."updated_at",
                "projects"."creator_id",
                "projects"."namespace_id",
                "projects"."last_activity_at",
                "projects"."import_url",
                "projects"."visibility_level",
                "projects"."archived",
                "projects"."avatar",
                "projects"."merge_requests_template",
                "projects"."star_count",
                "projects"."merge_requests_rebase_enabled",
                "projects"."import_type",
                "projects"."import_source",
                "projects"."approvals_before_merge",
                "projects"."reset_approvals_on_push",
                "projects"."merge_requests_ff_only_enabled",
                "projects"."issues_template",
                "projects"."mirror",
                "projects"."mirror_user_id",
                "projects"."shared_runners_enabled",
                "projects"."runners_token",
                "projects"."build_coverage_regex",
                "projects"."build_allow_git_fetch",
                "projects"."build_timeout",
                "projects"."mirror_trigger_builds",
                "projects"."pending_delete",
                "projects"."public_builds",
                "projects"."last_repository_check_failed",
                "projects"."last_repository_check_at",
                "projects"."only_allow_merge_if_pipeline_succeeds",
                "projects"."has_external_issue_tracker",
                "projects"."repository_storage",
                "projects"."repository_read_only",
                "projects"."request_access_enabled",
                "projects"."has_external_wiki",
                "projects"."ci_config_path",
                "projects"."lfs_enabled",
                "projects"."description_html",
                "projects"."only_allow_merge_if_all_discussions_are_resolved",
                "projects"."repository_size_limit",
                "projects"."printing_merge_request_link_enabled",
                "projects"."auto_cancel_pending_pipelines",
                "projects"."service_desk_enabled",
                "projects"."cached_markdown_version",
                "projects"."delete_error",
                "projects"."last_repository_updated_at",
                "projects"."disable_overriding_approvers_per_merge_request",
                "projects"."storage_version",
                "projects"."resolve_outdated_diff_discussions",
                "projects"."remote_mirror_available_overridden",
                "projects"."only_mirror_protected_branches",
                "projects"."pull_mirror_available_overridden",
                "projects"."jobs_cache_index",
                "projects"."external_authorization_classification_label",
                "projects"."mirror_overwrites_diverged_branches",
                "projects"."pages_https_only",
                "projects"."external_webhook_token",
                "projects"."packages_enabled",
                "projects"."merge_requests_author_approval",
                "projects"."pool_repository_id",
                "projects"."runners_token_encrypted",
                "projects"."bfg_object_map",
                "projects"."detected_repository_languages",
                "projects"."merge_requests_disable_committers_approval",
                "projects"."require_password_to_approve",
                "projects"."emails_disabled",
                "projects"."max_pages_size",
                "projects"."max_artifacts_size",
                "projects"."remove_source_branch_after_merge",
                "projects"."marked_for_deletion_at",
                "projects"."marked_for_deletion_by_user_id",
                "projects"."autoclose_referenced_issues",
                "projects"."suggestion_commit_message",
                "projects"."project_namespace_id"
            FROM
                "projects"
            WHERE
                "projects"."id" IN ( SELECT DISTINCT
                        "events"."project_id"
                    FROM
                        "events"
                    WHERE (action = 5
                        OR (target_type IN ('MergeRequest', 'Issue')
                            AND action IN (1, 3, 7))
                        OR (target_type = 'Note'
                            AND action = 6))
                    AND "events"."author_id" = 1
                    AND (created_at > '2020-11-26 17:14:51.444475'))
                AND "projects"."visibility_level" IN (0, 10, 20))) projects
ORDER BY
    "projects"."id" DESC

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Closes #24137 (closed)

Edited by Dustin Eckhardt

Merge request reports