-
Notifications
You must be signed in to change notification settings - Fork 5.6k
feat: allow private project views for the owning user #9686
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
base: preview
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,7 +64,6 @@ class Meta: | |
| "project", | ||
| "query", | ||
| "owned_by", | ||
| "access", | ||
| "is_locked", | ||
| ] | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,8 +24,11 @@ import { EViewAccess, EIssuesStoreType } from "@plane/types"; | |
| import { Input, TextArea } from "@plane/ui"; | ||
| import { getComputedDisplayFilters, getComputedDisplayProperties, getTabIndex } from "@plane/utils"; | ||
| // components | ||
| import { AccessField } from "@/components/common/access-field"; | ||
| import { DisplayFiltersSelection, FiltersDropdown } from "@/components/issues/issue-layouts/filters"; | ||
| import { WorkItemFiltersRow } from "@/components/work-item-filters/filters-row"; | ||
| // helpers | ||
| import { VIEW_ACCESS_SPECIFIERS } from "@/helpers/views.helper"; | ||
| // hooks | ||
| import { useProject } from "@/hooks/store/use-project"; | ||
| import { usePlatformOS } from "@/hooks/use-platform-os"; | ||
|
|
@@ -79,6 +82,8 @@ export const ProjectViewForm = observer(function ProjectViewForm(props: Props) { | |
| // derived values | ||
| const projectDetails = getProjectById(projectId); | ||
| const logoValue = watch("logo_props"); | ||
| const accessValue = watch("access"); | ||
| const i18nAccessLabel = VIEW_ACCESS_SPECIFIERS.find((access) => access.key === accessValue)?.i18n_label; | ||
| const workItemFilters: IIssueFilters = { | ||
| richFilters: getValues("rich_filters"), | ||
| displayFilters: getValues("display_filters"), | ||
|
|
@@ -278,19 +283,36 @@ export const ProjectViewForm = observer(function ProjectViewForm(props: Props) { | |
| </div> | ||
| </div> | ||
| </div> | ||
| <div className="flex items-center justify-end gap-2 border-t-[0.5px] border-subtle px-5 py-4"> | ||
| <Button variant="secondary" size="lg" onClick={handleClose} tabIndex={getIndex("cancel")}> | ||
| {t("common.cancel")} | ||
| </Button> | ||
| <Button variant="primary" size="lg" type="submit" tabIndex={getIndex("submit")} loading={isSubmitting}> | ||
| {data | ||
| ? isSubmitting | ||
| ? t("common.updating") | ||
| : t("view.update.label") | ||
| : isSubmitting | ||
| ? t("common.creating") | ||
| : t("view.create.label")} | ||
| </Button> | ||
| <div className="flex items-center justify-between gap-2 border-t-[0.5px] border-subtle px-5 py-4"> | ||
| <div className="flex items-center gap-2"> | ||
| <Controller | ||
| control={control} | ||
| name="access" | ||
| render={({ field: { value, onChange } }) => ( | ||
| <AccessField | ||
| onChange={onChange} | ||
| value={value ?? EViewAccess.PUBLIC} | ||
| accessSpecifiers={VIEW_ACCESS_SPECIFIERS} | ||
| isMobile={isMobile} | ||
| /> | ||
|
Comment on lines
+288
to
+297
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 8 'function Tooltip|const Tooltip|aria-label|aria-labelledby|aria-describedby|tooltipContent' \
apps/web packages --glob '*.tsx' --glob '*.ts'Repository: makeplane/plane Length of output: 50372 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- AccessField ---'
cat -n apps/web/core/components/common/access-field.tsx | sed -n '1,100p'
printf '%s\n' '--- Tooltip imports and definitions ---'
rg -n -C 5 '(^|[^[:alnum:]_])Tooltip([^[:alnum:]_]|$)' apps/web/core/components/common packages/propel --glob '*.tsx' --glob '*.ts' \
| head -n 240Repository: makeplane/plane Length of output: 21285 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Tooltip implementation ---'
cat -n packages/propel/src/tooltip/root.tsx | sed -n '1,240p'
printf '%s\n' '--- Tooltip barrel ---'
fd -i 'tooltip' packages/propel/src --type f | sortRepository: makeplane/plane Length of output: 3677 🌐 Web query:
💡 Result: In Base UI for React, tooltips are intended to be visual-only, supplementary elements and should not be used as the primary mechanism for labeling interactive elements [1][2]. Official documentation explicitly advises against relying on aria-describedby to link tooltips to their triggers [3]. To maintain accessibility: 1. Provide an Accessible Name: The Tooltip.Trigger should always have an accessible name (e.g., via aria-label or visible text) that describes the element's purpose, regardless of the tooltip content [1][2]. 2. Avoid aria-describedby for Tooltips: Screen readers may inconsistently announce tooltip content when aria-describedby is used, potentially causing redundant or confusing announcements [3][4]. Furthermore, because tooltips are not accessible to all input modalities (such as touch), critical information should never be hidden inside one [1][2]. 3. Use Alternatives for Essential Content: If the information provided by the tooltip is essential for the user to understand or interact with the element, use a more persistent component like a Popover instead [3][1][2]. The Base UI team has explicitly noted that they do not include aria-describedby as a default requirement for the Tooltip component because tooltips are meant to be visual enhancements, not a source of screen reader information [3]. If your use case strictly requires providing extra, non-essential description, you may manually apply aria-describedby, but you should be aware of the potential for redundant screen reader announcements [3]. Citations:
Give each access button an accessible name.
🤖 Prompt for AI Agents |
||
| )} | ||
| /> | ||
| <h6 className="text-11 font-medium">{t(i18nAccessLabel || "")}</h6> | ||
| </div> | ||
| <div className="flex items-center justify-end gap-2"> | ||
| <Button variant="secondary" size="lg" onClick={handleClose} tabIndex={getIndex("cancel")}> | ||
| {t("common.cancel")} | ||
| </Button> | ||
| <Button variant="primary" size="lg" type="submit" tabIndex={getIndex("submit")} loading={isSubmitting}> | ||
| {data | ||
| ? isSubmitting | ||
| ? t("common.updating") | ||
| : t("view.update.label") | ||
| : isSubmitting | ||
| ? t("common.creating") | ||
| : t("view.create.label")} | ||
| </Button> | ||
| </div> | ||
| </div> | ||
| </form> | ||
| ); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Apply the guest visibility rule before returning 404.
get_queryset()does not applyguest_view_all_features. Therefore, a guest with that flag disabled can match a public view they do not own and reach Lines 326-340, which return403. This exposes the view’s existence and conflicts with the required404response for views that are not visible to the user. Apply the guest restriction in the queryset or return the same404response from that branch.🤖 Prompt for AI Agents