search

hughbris / grav-plugin-pushy

Push with Git to publish changes to your production environment
MIT License
3 stars 1 forks source link

Errors viewing and publishing deleted/renamed files #24

Closed hughbris closed 1 year ago

hughbris commented 1 year ago

When files/pages are deleted as part of editing a Grav site, Pushy users see the following error in a strip banner in Admin:

"No valid response from server."

None of the changed files are listed, so users cannot publish. The numerical badge showing the number of changes in the Admin menu does show a correct count of changed files.

hughbris commented 1 year ago

The problem is in classes/RequestHandler.php at line 143, where the page object cannot be loaded – is null because the page has been deleted – and the method attempts to load its page title. Grav throws an error and returns an invalid JSON response to the AJAX listing request.

The branch hotfix/issue-24 has been applied and only works around this issue in a kludgey fashion. It is unlikely to be a comprehensive fix either.

I'm seeking a robust way to handle deleted files. This may require some refactoring. I'm pausing to consider how best to go about this.

pamtbaau commented 1 year ago

Deleting a page has indeed not been considered when writing/testing RequestHandler...

I'm seeking a robust way to handle deleted files.

What issues do you foresee?

Some issues I foresee when deleting/renaming:

Delete:

Rename:

hughbris commented 1 year ago

Deleting a page has indeed not been considered when writing/testing RequestHandler...

I'm seeking a robust way to handle deleted files.

What issues do you foresee?

It's funny, I was composing an email where I pretty much wanted to outline the same points, as I've been giving it a lot of thought but wasn't confident enough in expressing them coherently here. You've done a pretty good job though.

Some issues I foresee when deleting/renaming:

Delete:

* Git only contains the path to the item

* Grav does not retain the Page object after deleting the page

* The UI should include some hints to the user about the status of the entry listed (New, Modified, Deleted)

Rename:

* Git has now two entries and AFAIK without a way to relate the two

  * A deleted item and its path
  * A new item and its path

* Grav has only a single Page object containing the new information.

* The UI should provide some hints about the status of each item, but how to relate the two items?

* Ideally, deleted and new item should be committed/reverted together, but how can these two files be related as one single unit?

We're hitting a hard intersection here between providing a great, intuitive UX, keeping the codebase simple, and the limitations of what Git can tell us about the working tree.

Better rename detection would really help here.

A long time ago when thinking about how this plugin should work with repositories, I imagined hooking into Admin save events and staging changes from there. That would allow git mv and git rm directives to be issued. However, staging when saving gets messy when you want to allow users to only deploy some changes (as we do currently).

I am also wondering if it's possible to exploit Git better and create a Git commit on every save, which editors can then review as a sequence of (save) steps they can choose to "Publish" rather than a set of files. (hmm, the automatic commit function would need to come up with a smart way to describe each save action.) This is an early stage concept. I'm just mentioning it to demonstrate how this might need to be be re-architected. If this approach works, it opens up the possibility of scheduling updates too!

I don't want to clutter this issue with unformed ideas. Perhaps we should focus on improving the UX here (labeling and grouping if possible) and a better way will become apparent. Thanks for outlining the requirements so well!