Page MenuHomePhabricator

Replace EditPage to decouple saving logic from edit processing and user interface
Closed, DuplicatePublic


EditPage is very long and overly complicated, it even carries a Surgeon General's Warning...
It combines the logic for attempting save of a page with the processing of the edit request and the outputting of the user interface.
It is in dire need of refactoring, as evidenced by the multiple code comments there. It is going to be harder and harder to maintain in the future if we don't.
However refactoring from within would make it difficult to keep the hooks working.
I believe the class is not salvageable and we need to replace it entirely.
We'll need to write hooks for the new classes and keep the old ones until extensions no longer create EditPage instances.

I propose a new architecture as follows:

  • an Edit class to model an edit
  • an EditAttempt class that make the internal save attempt based on an Edit object
  • an Editor class that controls the user interface for editing
  • an EditFacilitator class that processes a form data into an Edit object, and mediates saving via EditAttempt
  • an EditView class that contains the UI elements for an editor class
  • and a static utility class EditUtilities.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 16 2017, 11:12 PM
Ebe123 added a subscriber: Ebe123.Feb 17 2017, 12:45 AM

Yes, I'll merge it.

Cenarium updated the task description. (Show Details)Feb 25 2017, 4:17 PM
Cenarium updated the task description. (Show Details)Feb 25 2017, 4:20 PM