Feature kpoland public private setting on dataset #217

klpoland · 2025-11-06T19:26:02Z

semanticdiff-com · 2025-11-06T19:26:05Z

Review changes with

Changed Files

File	Status
gateway/sds_gateway/users/views.py	41% smaller
gateway/sds_gateway/templates/users/group_captures.html	33% smaller
gateway/sds_gateway/static/js/actions/DetailsActionManager.js	19% smaller
gateway/sds_gateway/static/js/dataset/DatasetCreationHandler.js	12% smaller
gateway/sds_gateway/templates/users/partials/files_management.html	6% smaller
gateway/sds_gateway/static/js/dataset/DatasetModeManager.js	6% smaller
gateway/sds_gateway/templates/users/partials/captures_management.html	5% smaller
gateway/sds_gateway/static/js/dataset/DatasetEditingHandler.js	4% smaller
gateway/sds_gateway/templates/users/partials/step_4.html	3% smaller
gateway/sds_gateway/users/forms.py	1% smaller
gateway/sds_gateway/api_methods/migrations/0017_convert_dataset_authors_to_object_format.py	0% smaller
gateway/sds_gateway/api_methods/serializers/dataset_serializers.py	0% smaller
gateway/sds_gateway/static/css/components.css	0% smaller
gateway/sds_gateway/static/js/actions/PublishActionManager.js	0% smaller
gateway/sds_gateway/static/js/search/AssetSearchHandler.js	0% smaller
gateway/sds_gateway/templates/users/dataset_list.html	Unsupported file format
gateway/sds_gateway/templates/users/partials/publish_dataset_modal.html	Unsupported file format
gateway/sds_gateway/templates/users/partials/review_create_dataset.html	0% smaller
gateway/sds_gateway/templates/users/partials/step_1.html	0% smaller
gateway/sds_gateway/templates/users/partials/step_5.html	0% smaller
gateway/sds_gateway/users/urls.py	0% smaller

Copilot

Pull Request Overview

This PR implements a public/private setting feature for datasets, allowing dataset owners and co-owners to publish datasets and control their visibility. The feature includes comprehensive UI changes for dataset creation/editing workflows and adds backend validation to prevent editing of published or public datasets.

Key Changes

Adds is_public boolean field to datasets with UI controls for public/private visibility selection
Implements a new publishing workflow with a dedicated step in the dataset creation/editing wizard
Creates a publish modal and API endpoint for updating dataset status and visibility from the dataset list
Prevents editing of datasets that are published (status=final) or made public

Reviewed Changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
gateway/sds_gateway/users/views.py	Adds `PublishDatasetView` for handling publish requests, refactors permission validation with status/public checks, and updates dataset creation to handle `is_public` field
gateway/sds_gateway/users/urls.py	Adds URL route for the new publish dataset endpoint
gateway/sds_gateway/users/forms.py	Adds `is_public` BooleanField and changes status field to HiddenInput widget
gateway/sds_gateway/templates/users/partials/step_4.html	New step for publishing info with toggles for status and visibility
gateway/sds_gateway/templates/users/partials/step_5.html	Moves review/create content to step 5 (previously step 4)
gateway/sds_gateway/templates/users/partials/step_1.html	Removes status field from basic info step (moved to publishing step)
gateway/sds_gateway/templates/users/partials/review_create_dataset.html	Removes status display and adds publishing alerts container
gateway/sds_gateway/templates/users/partials/publish_dataset_modal.html	New modal for publishing datasets from the dataset list
gateway/sds_gateway/templates/users/group_captures.html	Updates step tabs and passes dataset status/public state to JavaScript
gateway/sds_gateway/templates/users/dataset_list.html	Adds public/private icons, publish button in dropdown, and conditional edit button visibility
gateway/sds_gateway/static/js/dataset/DatasetModeManager.js	Major updates to handle 5-step workflow, publishing info UI, and submit button styling based on publish status
gateway/sds_gateway/static/js/dataset/DatasetCreationHandler.js	Updates step indices for 5-step workflow
gateway/sds_gateway/static/js/dataset/DatasetEditingHandler.js	Replaces inline opacity styles with CSS class for disabled elements
gateway/sds_gateway/static/js/actions/PublishActionManager.js	New manager class for handling publish modal interactions and API calls
gateway/sds_gateway/static/js/actions/DetailsActionManager.js	Updates status display to use badge instead of plain text
gateway/sds_gateway/static/js/search/AssetSearchHandler.js	Adds null check for search form (handles public datasets without search)
gateway/sds_gateway/static/css/components.css	Adds disabled-element and disable-events CSS classes, textarea padding fix
gateway/sds_gateway/api_methods/serializers/dataset_serializers.py	Adds `is_public` field to dataset serializer
gateway/sds_gateway/api_methods/migrations/0017_convert_dataset_authors_to_object_format.py	Removes blank lines (formatting only)
gateway/sds_gateway/templates/users/partials/files_management.html	Adds id to files section for JavaScript targeting
gateway/sds_gateway/templates/users/partials/captures_management.html	Adds id to captures section for JavaScript targeting

Comments suppressed due to low confidence (1)

gateway/sds_gateway/static/css/components.css:185

Duplicate CSS rule: The .author-item class is defined twice (lines 123-125 and 177-179) with slightly different styles. The second definition will override the first one. These should be consolidated into a single rule to avoid confusion and ensure predictable styling.

/* Author management styling */
.author-item {
	transition: all 0.3s ease;
}

.author-item:hover {
	background-color: var(--bs-gray-100);
}

.author-item.marked-for-removal .form-control {
	background-color: var(--bs-gray-100);
	border-color: var(--bs-gray-300);
	color: var(--bs-gray-600);
}

.author-name-input,
.author-orcid-input {
	font-size: 0.9rem;
}

.author-orcid-input {
	font-family: "Courier New", monospace;
}

/* Permission-based styling */
.readonly-field {
	background-color: var(--bs-gray-100);
	border: 1px solid var(--bs-gray-300);
	color: var(--bs-gray-600);
}

.readonly-field:focus {
	background-color: var(--bs-gray-100);
	border-color: var(--bs-gray-300);
	box-shadow: none;
}

.readonly-row {
	background-color: var(--bs-gray-100);
	opacity: 0.8;
}
.readonly-row td {
	color: var(--bs-gray-600);
}
.readonly-row .badge {
	opacity: 0.7;
}

/* Disabled state styling */
.disabled-element {
	opacity: 0.5;
	pointer-events: none;
}

/* Authors management styling */
.author-item {
	transition: all 0.2s ease;
}

.author-item:hover {
	background-color: var(--bs-gray-100);
	padding: 0.5rem;
	border-radius: 0.375rem;
}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-14T21:14:07Z

gateway/sds_gateway/templates/users/partials/step_5.html

@@ -0,0 +1,3 @@
+<div class="tab-pane fade" id="step4">


The id attribute should be "step5" instead of "step4". The file is named step_5.html and is meant to be the 5th step, but uses id="step4" which conflicts with the actual step 4 (publishing info) defined in step_4.html. This will cause JavaScript step navigation to fail as both steps will have the same ID.

Suggested change

<div class="tab-pane fade" id="step4">

<div class="tab-pane fade" id="step5">

Copilot · 2025-11-14T21:14:08Z

gateway/sds_gateway/templates/users/partials/publish_dataset_modal.html

+            <div class="modal-body">
+                <h5 class="card-title mb-4">Dataset Publishing</h5>
+                <!-- Published Indicator (only shown for final datasets) -->
+                {% if dataset.dataset.status == 'final' %}


Inconsistent property access pattern: dataset.dataset.status is used here but dataset.is_public is used on line 27. This suggests that dataset is a wrapper object with a nested dataset property for the actual dataset model, but is_public is accessed directly on the wrapper. This inconsistency could lead to bugs if the structure changes. Consider using dataset.dataset.is_public for consistency, or document why the different access patterns are intentional.

Copilot · 2025-11-14T21:14:08Z

gateway/sds_gateway/static/js/actions/PublishActionManager.js

+	 * @param {HTMLElement} publishToggle - The publish toggle checkbox
+	 * @param {HTMLElement} visibilitySection - The visibility section element
+	 * @param {HTMLElement} statusBadge - The status badge element
+	 * @param {string} datasetUuid - Dataset UUID


The JSDoc comment incorrectly lists datasetUuid as a parameter, but the method signature for handlePublishToggleChange doesn't include datasetUuid as a parameter. The parameter list should be updated to remove this.

Suggested change

* @param {string} datasetUuid - Dataset UUID

Copilot · 2025-11-14T21:14:08Z

gateway/sds_gateway/users/views.py

+        if dataset.status == DatasetStatus.FINAL:
+            messages.error(request, "This dataset is published and cannot be edited.")
+            return redirect("users:dataset_list")
+


The backend validation only checks if the dataset status is FINAL, but the frontend (dataset_list.html line 171) also prevents editing public datasets. This inconsistency creates a potential security issue - a user could bypass the frontend restriction and still edit a public dataset through direct API calls if it's not FINAL. Add a check for dataset.is_public to be consistent with the frontend restrictions:

# Check if dataset is final (published) or public - cannot be edited if dataset.status == DatasetStatus.FINAL: messages.error(request, "This dataset is published and cannot be edited.") return redirect("users:dataset_list") if dataset.is_public: messages.error(request, "This dataset is public and cannot be edited.") return redirect("users:dataset_list")

Suggested change

# Check if dataset is public - cannot be edited

if dataset.is_public:

messages.error(request, "This dataset is public and cannot be edited.")

return redirect("users:dataset_list")

Copilot · 2025-11-14T21:14:09Z

gateway/sds_gateway/users/views.py

+
+        # Get status and is_public from request
+        status_value = request.POST.get("status")
+        is_public_value = json.loads(request.POST.get("is_public"))


The json.loads() call on line 2477 can raise a JSONDecodeError if the is_public value is not valid JSON or is None. This should be wrapped in a try-except block to handle parsing errors gracefully and return an appropriate error response.

Consider:

try: is_public_value = json.loads(request.POST.get("is_public")) except (json.JSONDecodeError, TypeError, ValueError): return JsonResponse({"success": False, "error": "Invalid is_public value."}, status=400)

Suggested change

is_public_value = json.loads(request.POST.get("is_public"))

try:

is_public_value = json.loads(request.POST.get("is_public"))

except (json.JSONDecodeError, TypeError, ValueError):

return JsonResponse({"success": False, "error": "Invalid is_public value."}, status=400)

Copilot · 2025-11-14T21:14:09Z

gateway/sds_gateway/static/js/dataset/DatasetModeManager.js

 	}

+	/**
+	 * Check if this is an existing final-status datase


Typo in the comment: "datase" should be "dataset"

Suggested change

* Check if this is an existing final-status datase

* Check if this is an existing final-status dataset

Copilot · 2025-11-14T21:14:10Z

gateway/sds_gateway/api_methods/serializers/dataset_serializers.py

+    def get_is_public(self, obj):
+        """Check if the dataset is public."""
+        return obj.is_public


The get_is_public method is redundant. Since is_public is already defined as a BooleanField on line 16, it will automatically serialize the model's is_public attribute. The get_is_public method (lines 48-50) simply returns the same value without any transformation. This method should be removed, and the field declaration on line 16 is sufficient.

Suggested change

def get_is_public(self, obj):

"""Check if the dataset is public."""

return obj.is_public

Copilot · 2025-11-14T21:14:10Z

gateway/sds_gateway/static/js/actions/DetailsActionManager.js

+	updateStatusBadge(modal, status) {
+		const statusContainer = modal.querySelector(".dataset-details-status");
+		const statusText = status[0].toUpperCase() + status.slice(1);
+		const statusClass = status === "final" ? "success" : "secondary";
+		if (statusContainer) {
+			statusContainer.innerHTML = `<span class="badge bg-${statusClass}">${statusText}</span>`;
+		}
+	}


The updateStatusBadge method will throw a runtime error if status is null, undefined, or an empty string, because it attempts to access status[0] and call status.slice(1) without validation. Add a check for valid status value before string manipulation.

Suggested fix:

updateStatusBadge(modal, status) { const statusContainer = modal.querySelector(".dataset-details-status"); if (!status || typeof status !== 'string') { if (statusContainer) { statusContainer.innerHTML = `<span class="badge bg-secondary">Unknown</span>`; } return; } const statusText = status[0].toUpperCase() + status.slice(1); const statusClass = status === "final" ? "success" : "secondary"; if (statusContainer) { statusContainer.innerHTML = `<span class="badge bg-${statusClass}">${statusText}</span>`; } }

Copilot · 2025-11-14T21:14:10Z

gateway/sds_gateway/static/js/dataset/DatasetModeManager.js

+	updateSubmitButton() {
+		const submitBtn = document.getElementById("submitForm");
+		const publishToggle = document.getElementById("publish-dataset-toggle");
+		const publicOption = document.getElementById("public-option");


Unused variable publicOption.

Suggested change

const publicOption = document.getElementById("public-option");

gateway/sds_gateway/templates/users/dataset_list.html

klpoland self-assigned this Nov 6, 2025

klpoland added feature New feature or request gateway Gateway component javascript Pull requests that update non-trivial javascript code labels Nov 6, 2025

klpoland force-pushed the feature-kpoland-public-private-setting-on-dataset branch 2 times, most recently from 0e4ff1c to 167ebad Compare November 13, 2025 16:01

klpoland force-pushed the feature-kpoland-public-private-setting-on-dataset branch from 167ebad to 8c75f96 Compare November 14, 2025 18:09

klpoland marked this pull request as ready for review November 14, 2025 20:32

klpoland requested a review from lucaspar November 14, 2025 20:32

klpoland added 9 commits November 14, 2025 15:34

cleaning up implementation (1)

3cab2b4

first pass

5798d29

public dataset settings

1bf225b

adding new publishing step and ui triggers

957d582

setting public/private UI events/logic

aec0e7f

break publishing step into a partial template

09d37db

add publish action on dataset list

87bea4e

add publishing view to urls

f67db6a

updating details, status display/update logic

d43f3c6

klpoland force-pushed the feature-kpoland-public-private-setting-on-dataset branch from 05d7301 to d43f3c6 Compare November 14, 2025 20:34

lucaspar requested a review from Copilot November 14, 2025 21:09

Copilot started reviewing on behalf of lucaspar November 14, 2025 21:09 View session

Copilot finished reviewing on behalf of lucaspar November 14, 2025 21:12

Copilot AI reviewed Nov 14, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Feature kpoland public private setting on dataset #217

Feature kpoland public private setting on dataset #217

Uh oh!

klpoland commented Nov 6, 2025 •

edited by lucaspar

Loading

Uh oh!

semanticdiff-com bot commented Nov 6, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Copilot AI Nov 14, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	<div class="tab-pane fade" id="step4">
	<div class="tab-pane fade" id="step5">

+        # Check if dataset is public - cannot be edited
+        if dataset.is_public:
+            messages.error(request, "This dataset is public and cannot be edited.")
+            return redirect("users:dataset_list")

	* Check if this is an existing final-status datase
	* Check if this is an existing final-status dataset

	def get_is_public(self, obj):
	"""Check if the dataset is public."""
	return obj.is_public

Feature kpoland public private setting on dataset #217

Are you sure you want to change the base?

Feature kpoland public private setting on dataset #217

Uh oh!

Conversation

klpoland commented Nov 6, 2025 • edited by lucaspar Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

semanticdiff-com bot commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Reviewed Changes

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

klpoland commented Nov 6, 2025 •

edited by lucaspar

Loading

semanticdiff-com bot commented Nov 6, 2025 •

edited

Loading