Merge pull request #21819 from marination/attachments-access

cypress/fixtures/sample_attachments/attachment-10.txt

@@ -0,0 +1 @@
+10

cypress/fixtures/sample_attachments/attachment-11.txt

@@ -0,0 +1 @@
+11

cypress/fixtures/sample_attachments/attachment-2.txt

@@ -0,0 +1 @@
+2

cypress/fixtures/sample_attachments/attachment-3.txt

@@ -0,0 +1 @@
+3

cypress/fixtures/sample_attachments/attachment-4.txt

@@ -0,0 +1 @@
+4

cypress/fixtures/sample_attachments/attachment-5.txt

@@ -0,0 +1 @@
+5

cypress/fixtures/sample_attachments/attachment-6.txt

@@ -0,0 +1 @@
+6

cypress/fixtures/sample_attachments/attachment-7.txt

@@ -0,0 +1 @@
+7

cypress/fixtures/sample_attachments/attachment-8.txt

@@ -0,0 +1 @@
+8

cypress/fixtures/sample_attachments/attachment-9.txt

@@ -0,0 +1 @@
+9

cypress/integration/sidebar.js

@@ -13,6 +13,27 @@ const verify_attachment_visibility = (document, is_private) => {
 	cy.get_open_dialog().findByRole("checkbox", { name: "Private" }).should(assertion);
 };
 
+const attach_file = (file, no_of_files = 1) => {
+	let files = [];
+	if (file) {
+		files = [file];
+	} else if (no_of_files > 1) {
+		// attach n files
+		files = [...Array(no_of_files)].map(
+			(el, idx) =>
+				"cypress/fixtures/sample_attachments/attachment-" +
+				(idx + 1) +
+				(idx == 0 ? ".jpg" : ".txt")
+		);
+	}
+
+	cy.findByRole("button", { name: "Attach File" }).click();
+	cy.get_open_dialog().find(".file-upload-area").selectFile(files, {
+		action: "drag-drop",
+	});
+	cy.get_open_dialog().findByRole("button", { name: "Upload" }).click();
+};
+
 context("Sidebar", () => {
 	before(() => {
 		cy.visit("/login");
@@ -35,6 +56,36 @@ context("Sidebar", () => {
 		verify_attachment_visibility("blog-post/test-blog-attachment-post", false);
 	});
 
+	it("Verify attachment accessibility UX", () => {
+		cy.call("frappe.tests.ui_test_helpers.create_todo_with_attachment_limit", {
+			description: "Sidebar Attachment Access Test ToDo",
+		}).then((todo) => {
+			cy.visit(`/app/todo/${todo.message.name}`);
+
+			// explore icon btn should be hidden as there are no attachments
+			cy.get(".explore-btn").should("be.hidden");
+
+			attach_file("cypress/fixtures/sample_image.jpg");
+			cy.get(".explore-btn").should("be.visible");
+			cy.get(".show-all-btn").should("be.hidden");
+
+			// attach 10 images
+			attach_file(null, 10);
+			cy.get(".show-all-btn").should("be.visible");
+
+			// attach 1 more image to reach attachment limit
+			attach_file("cypress/fixtures/sample_attachments/attachment-11.txt");
+			cy.get(".explore-full-btn").should("be.visible");
+			cy.get(".attachments-actions").should("be.hidden");
+			cy.get(".explore-btn").should("be.hidden");
+
+			// test "Show All" button
+			cy.get(".attachment-row").should("have.length", 10);
+			cy.get(".show-all-btn").click();
+			cy.get(".attachment-row").should("have.length", 12);
+		});
+	});
+
 	it('Test for checking "Assigned To" counter value, adding filter and adding & removing an assignment', () => {
 		cy.call("frappe.tests.ui_test_helpers.create_todo", {
 			description: "Sidebar Attachment ToDo",

frappe/core/doctype/file/file.js

@@ -27,8 +27,7 @@ frappe.ui.form.on("File", {
 
 	preview_file: function (frm) {
 		let $preview = "";
-		let file_name = frm.doc.file_name.split("?")[0];
-		let file_extension = file_name.split(".").pop()?.toLowerCase();
+		let file_extension = frm.doc.file_type.toLowerCase();
 
 		if (frappe.utils.is_image_file(frm.doc.file_url)) {
 			$preview = $(`<div class="img_preview">

frappe/core/doctype/file/file.json

@@ -8,6 +8,8 @@
  "field_order": [
   "file_name",
   "is_private",
+  "column_break_7jmm",
+  "file_type",
   "preview",
   "preview_html",
   "section_break_5",
@@ -169,13 +171,25 @@
    "fieldtype": "Check",
    "label": "Uploaded To Google Drive",
    "read_only": 1
+  },
+  {
+   "fieldname": "column_break_7jmm",
+   "fieldtype": "Column Break"
+  },
+  {
+   "fieldname": "file_type",
+   "fieldtype": "Data",
+   "in_list_view": 1,
+   "in_standard_filter": 1,
+   "label": "File Type",
+   "read_only": 1
   }
  ],
  "force_re_route_to_default_view": 1,
  "icon": "fa fa-file",
  "idx": 1,
  "links": [],
- "modified": "2023-08-02 09:43:51.178011",
+ "modified": "2023-08-02 09:43:51.178012",
  "modified_by": "Administrator",
  "module": "Core",
  "name": "File",

frappe/core/doctype/file/file.py

@@ -44,6 +44,7 @@ class File(Document):
 		content_hash: DF.Data | None
 		file_name: DF.Data | None
 		file_size: DF.Int
+		file_type: DF.Data | None
 		file_url: DF.Code | None
 		folder: DF.Link | None
 		is_attachments_folder: DF.Check
@@ -86,6 +87,7 @@ class File(Document):
 		self.set_folder_name()
 		self.set_file_name()
 		self.validate_attachment_limit()
+		self.set_file_type()
 
 		if self.is_folder:
 			return
@@ -330,6 +332,17 @@ class File(Document):
 		elif not self.is_home_folder:
 			self.folder = "Home"
 
+	def set_file_type(self):
+		if self.is_folder:
+			return
+
+		file_type = mimetypes.guess_type(self.file_name)[0]
+		if not file_type:
+			return
+
+		file_extension = mimetypes.guess_extension(file_type)
+		self.file_type = file_extension.lstrip(".").upper() if file_extension else None
+
 	def validate_file_on_disk(self):
 		"""Validates existence file"""
 		full_path = self.get_full_path()

frappe/patches.txt

@@ -228,3 +228,4 @@ execute:frappe.delete_doc_if_exists("Workspace", "Customization")
 execute:frappe.db.set_single_value("Document Naming Settings", "default_amend_naming", "Amend Counter")
 execute:frappe.delete_doc_if_exists("DocType", "Error Snapshot")
 frappe.patches.v15_0.move_event_cancelled_to_status
+frappe.patches.v15_0.set_file_type

frappe/patches/v15_0/set_file_type.py

@@ -0,0 +1,30 @@
+import mimetypes
+
+import frappe
+
+
+def execute():
+	"""Set 'File Type' for all files based on file extension."""
+	files = frappe.db.get_all(
+		"File",
+		fields=["name", "file_name", "file_url"],
+		filters={"is_folder": 0, "file_type": ("is", "not set")},
+	)
+
+	frappe.db.auto_commit_on_many_writes = 1
+
+	for file in files:
+		file_extension = get_file_extension(file.file_name or file.file_url)
+		if file_extension:
+			frappe.db.set_value("File", file.name, "file_type", file_extension, update_modified=False)
+
+	frappe.db.auto_commit_on_many_writes = 0
+
+
+def get_file_extension(file_name):
+	file_type = mimetypes.guess_type(file_name)[0]
+	if not file_type:
+		return None
+
+	file_extension = mimetypes.guess_extension(file_type)
+	return file_extension.lstrip(".").upper() if file_extension else None

frappe/public/js/frappe/form/sidebar/attachments.js

@@ -1,9 +1,12 @@
 // Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
 // MIT License. See license.txt
-
 frappe.ui.form.Attachments = class Attachments {
 	constructor(opts) {
 		$.extend(this, opts);
+
+		this.attachments_page_length = 10; // show n attachments initially
+		this.show_all_attachments = false;
+
 		this.make();
 	}
 	make() {
@@ -11,7 +14,16 @@ frappe.ui.form.Attachments = class Attachments {
 		this.parent.find(".add-attachment-btn").click(function () {
 			me.new_attachment();
 		});
-		this.add_attachment_wrapper = this.parent.find(".add-attachment-btn");
+
+		this.parent.find(".explore-btn").click(() => {
+			frappe.open_in_new_tab = true;
+			frappe.set_route("List", "File", {
+				attached_to_doctype: this.frm.doctype,
+				attached_to_name: this.frm.docname,
+			});
+		});
+
+		this.add_attachment_wrapper = this.parent.find(".attachments-actions");
 		this.attachments_label = this.parent.find(".attachments-label");
 	}
 	max_reached(raise_exception = false) {
@@ -31,8 +43,6 @@ frappe.ui.form.Attachments = class Attachments {
 		return false;
 	}
 	refresh() {
-		var me = this;
-
 		if (this.frm.doc.__islocal) {
 			this.parent.toggle(false);
 			return;
@@ -42,12 +52,66 @@ frappe.ui.form.Attachments = class Attachments {
 
 		var max_reached = this.max_reached();
 		this.add_attachment_wrapper.toggle(!max_reached);
+		this.setup_expanded_explore_button(max_reached);
 
 		// add attachment objects
 		var attachments = this.get_attachments();
-		if (attachments.length) {
+		this.render_attachments(attachments);
+		this.setup_show_all_button(attachments);
+	}
+
+	setup_expanded_explore_button(max_reached) {
+		if (!max_reached) {
+			this.parent.find(".explore-full-btn").addClass("hidden");
+			return;
+		}
+
+		this.parent.find(".explore-full-btn").removeClass("hidden");
+		this.parent.find(".explore-full-btn").click(() => {
+			frappe.set_route("List", "File", {
+				attached_to_doctype: this.frm.doctype,
+				attached_to_name: this.frm.docname,
+			});
+		});
+	}
+
+	setup_show_all_button(attachments) {
+		// show button if there is more to show and user has not clicked on "Show All"
+		let is_slicable = attachments.length > this.attachments_page_length;
+		let show = !this.show_all_attachments && is_slicable;
+
+		let show_all_btn = this.parent.find(".show-all-btn");
+		if (!show) {
+			show_all_btn.addClass("hidden");
+			return;
+		}
+
+		show_all_btn.removeClass("hidden");
+		show_all_btn.click(() => {
+			show_all_btn.addClass("hidden");
+			this.show_all_attachments = true;
+			this.refresh();
+		});
+	}
+
+	get_attachments() {
+		return this.frm.get_docinfo().attachments || [];
+	}
+
+	render_attachments(attachments) {
+		var me = this;
+		let attachments_to_render = attachments;
+
+		let is_slicable = attachments.length > this.attachments_page_length;
+		if (!this.show_all_attachments && is_slicable) {
+			// render last n attachments as they are at the top
+			let start = attachments.length - this.attachments_page_length;
+			attachments_to_render = attachments.slice(start, attachments.length);
+		}
+
+		if (attachments_to_render.length) {
 			let exists = {};
-			let unique_attachments = attachments.filter((attachment) => {
+			let unique_attachments = attachments_to_render.filter((attachment) => {
 				return Object.prototype.hasOwnProperty.call(exists, attachment.file_name)
 					? false
 					: (exists[attachment.file_name] = true);
@@ -55,13 +119,15 @@ frappe.ui.form.Attachments = class Attachments {
 			unique_attachments.forEach((attachment) => {
 				me.add_attachment(attachment);
 			});
-		} else {
+		}
+
+		if (!attachments.length) {
+			// If no attachments in totality
 			this.attachments_label.removeClass("has-attachments");
+			this.parent.find(".explore-btn").toggle(false); // hide explore icon button
 		}
 	}
-	get_attachments() {
-		return this.frm.get_docinfo().attachments || [];
-	}
+
 	add_attachment(attachment) {
 		var file_name = attachment.file_name;
 		var file_url = this.get_file_url(attachment);
@@ -101,8 +167,11 @@ frappe.ui.form.Attachments = class Attachments {
 
 		$(`<li class="attachment-row">`)
 			.append(frappe.get_data_pill(file_label, fileid, remove_action, icon))
-			.insertAfter(this.attachments_label.addClass("has-attachments"));
+			.insertAfter(this.add_attachment_wrapper);
+
+		this.parent.find(".explore-btn").toggle(true); // show explore icon button if hidden
 	}
+
 	get_file_url(attachment) {
 		var file_url = attachment.file_url;
 		if (!file_url) {

frappe/public/js/frappe/form/templates/form_sidebar.html

@@ -54,17 +54,43 @@
 	<li class="sidebar-label attachments-label">
 		<svg class="icon icon-sm"><use href="#icon-attachment"></use></svg>
 		{%= __("Attachments") %}
+
+		<li class="explore-full-btn hidden">
+			<button class="data-pill btn">
+				<span class="pill-label ellipsis">
+					{%= __("Explore Files") %}
+				</span>
+				<svg class="icon icon-sm">
+					<use href="#icon-projects"></use>
+				</svg>
+			</button>
+		</li>
+
+		<li class="attachments-actions">
+			<button class="data-pill btn add-attachment-btn">
+				<span class="pill-label ellipsis">
+					{%= __("Attach File") %}
+				</span>
+				<svg class="icon icon-sm">
+					<use href="#icon-add"></use>
+				</svg>
+			</button>
+
+			<button class="text-muted btn btn-default icon-btn explore-btn">
+				<svg class="icon icon-sm">
+					<use href="#icon-projects"></use>
+				</svg>
+			</button>
+		</li>
+
+
+		<li class="show-all-btn hidden">
+			<a href="" class="pill-label ellipsis">
+				{%= __("Show All") %}
+			</a>
+		</li>
 	</li>
-	<li class="add-attachment-btn">
-		<button class="data-pill btn">
-			<span class="pill-label ellipsis">
-				{%= __("Attach File") %}
-			</span>
-			<svg class="icon icon-sm">
-				<use href="#icon-add"></use>
-			</svg>
-		</button>
-	</li>
+
 </ul>
 <ul class="list-unstyled sidebar-menu form-reviews">
 	<li class="sidebar-label reviews-label">

frappe/public/js/frappe/views/file/file_view.js

@@ -317,6 +317,9 @@ frappe.views.FileView = class FileView extends frappe.views.ListView {
 			? `<div class="list-row-col ellipsis hidden-xs">
 					<span>${__("Size")}</span>
 				</div>
+				<div class="list-row-col ellipsis hidden-xs">
+					<span>${__("Type")}</span>
+				</div>
 				<div class="list-row-col ellipsis hidden-xs">
 					<span>${__("Created")}</span>
 				</div>`
@@ -368,6 +371,9 @@ frappe.views.FileView = class FileView extends frappe.views.ListView {
 			<div class="list-row-col ellipsis hidden-xs text-muted">
 				<span>${file_size}</span>
 			</div>
+			<div class="list-row-col ellipsis hidden-xs text-muted">
+				<span>${file.file_type || ""}</span>
+			</div>
 			<div class="list-row-col ellipsis hidden-xs text-muted">
 				<span>${this.get_creation_date(file)}</span>
 			</div>

frappe/public/scss/desk/sidebar.scss

@@ -389,7 +389,22 @@ body[data-route^="Module"] .main-menu {
 	display: inline-flex;
 }
 
-.add-attachment-btn,
+.attachments-actions {
+	display: flex;
+	gap: 5px;
+	max-width: 100%;
+}
+
+.explore-full-btn,
+.attachments-actions {
+	margin-bottom: var(--margin-md);
+}
+
+.show-all-btn {
+	margin-top: var(--margin-md);
+	text-align: center;
+}
+
 .shares,
 .followed-by {
 	max-width: 100%;

frappe/tests/ui_test_helpers.py

@@ -583,6 +583,15 @@ def create_todo(description):
 	return frappe.get_doc({"doctype": "ToDo", "description": description}).insert()
 
 
+@whitelist_for_tests
+def create_todo_with_attachment_limit(description):
+	from frappe.custom.doctype.property_setter.property_setter import make_property_setter
+
+	make_property_setter("ToDo", None, "max_attachments", 12, "int", for_doctype=True)
+
+	return frappe.get_doc({"doctype": "ToDo", "description": description}).insert()
+
+
 @whitelist_for_tests
 def create_admin_kanban():
 	if not frappe.db.exists("Kanban Board", "Admin Kanban"):