frappe / wiki

Free and Open Source Wiki built on top of Frappe
https://frappe.io/wiki
MIT License
219 stars 168 forks source link

fix: add new pages to sidebar after patch approval #247

Closed mgiadach closed 2 months ago

mgiadach commented 2 months ago

This PR addresses issue #244 (refer to the full description there).

Problem

New Wiki Pages can be published in two distinct ways: (i) immediately, if the creator holds the Wiki Approver role; or (ii) pending manual approval of the associated Wiki Page Patch if the creator lacks the Wiki Approver role.

The logic for this is embedded in the whitelisted update function within the wiki/doctype/wiki_report/wiki_report.py file. This function checks if the user possesses the submit perm for that doctype to determine whether to submit it immediately or not. There is an issue within this function: it expects to receive the new_sidebar_items argument from the API call, and subsequently stores it (at runtime) in the Wiki Page Patch object. However, this doctype lacks a corresponding field, thus the information is not persisted in the database. Consequently, when the doctype is not immediately submitted, this information is lost, preventing its later use in updating the sidebar.

Solution

In my opinion, there is a design flaw in how sidebar updates are currently managed. Nevertheless, I have chosen not to alter the existing mechanism within this PR (such a refactor could be pursued in a following PR). My solution leverages the new_sidebar_group property (an existing property of the doctype) to compute the position in the sidebar and to update it when the new_sidebar_items property is not present (preserving the original behavior).

mgiadach commented 2 months ago

Hi @BreadGenie @rmehta , could you please review this when you have a chance? Thank you for your time!

mgiadach commented 2 months ago

Hi @BreadGenie , just a friendly reminder to check this PR. In my opinion, it fixes a very annoying bug in wikis with many contributors that require approvals to publish new pages.