adobe / da-live

Dark Alley is a research project
https://da.live
Apache License 2.0
7 stars 11 forks source link

GH-108 - Versions & Audits #129

Closed auniverseaway closed 3 months ago

auniverseaway commented 3 months ago

Resolves: GH-108

Motivation and Context

How Has This Been Tested?

Manual E2E (for now)

https://ver--da-live--adobe.aem.live/edit#/aemsites/da-block-collection/drafts/cmillar/test https://ver--da-live--adobe.aem.live/edit#/aemsites/da-block-collection/drafts/cmillar/net-new-no-version-or-audit

Types of changes

Checklist:

aem-code-sync[bot] commented 3 months ago

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed. In case there are problems, just click a checkbox below to rerun the respective action.

Commits * [169c620](https://github.com/adobe/da-live/commit/169c620b464c48155160dfc0333b65ad3e4162dd) :white_check_mark: (latest)
aem-code-sync[bot] commented 3 months ago
Page Scores Audits Google
/edit PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
/edit PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
auniverseaway commented 3 months ago

Notes & quirks

  1. I'm not terribly excited about the hover on "history" to see you can close the history view, but I also don't want to have the previous gray menu because when we start showing editor + preview + versions, we'll have to make major UX changes.
  2. The create version API does not provide the location of the version created, so you cannot make a 1:1 entry without re-hitting the full list API. This means if you click restore on a version you just created (not a happy path), it will not work. To fix: the version API should return the location of the version on create.
  3. If you make no audit-able changes, but create a version, the previous version label will be wiped in favor of the new version. This can be confusing given that there is a new entry added to the list. If we can get item 2 solved, we can check against the existing list to see if that version got updated and act correctly. The alternative would be to just not overwrite the previous version... which may be less confusing overall. I make a version, I get a version... even if it's the same as the other one.
  4. We also do not know who is associated with the version: the 201 should come back with this info as well.
  5. Audit log seems un-predictable. Cannot tell if it is time based, character based, de-bounced with collab weirdly or what.

Todo

  1. Add cancel button upon new version view.
  2. Get version on preview built.
  3. Unit tests.
karlpauls commented 3 months ago

@auniverseaway, I agree about 2. - the 201 should return the url of the version in the Location header (/cc @bosschaert )

Regarding 3., I'm not sure I follow - if you version again, it will override the previous version so there shouldn't be a new entry in the list. I suppose we could make it a new version and just have it be duplicated content but that would be something purely to accommodate the ability to create versions with a label. Alternatively, we could fail if you attempt to create a version without any change to the previous one (would be a boolean change in da-admin).

Regarding 4., I guess we could make that some meta-data in the response body of the 201.

Regarding 5., not sure what you mean exactly. The audit log captures all changes in da-admin. If those came via collab, that means they can have more than one author and yes, they will be debounced (collab does wait up to a couple of seconds before writing back). Is that what you mean?

bosschaert commented 3 months ago

@auniverseaway, I agree about 2. - the 201 should return the url of the version in the Location header (/cc @bosschaert )

Regarding 4., I guess we could make that some meta-data in the response body of the 201.

Created https://github.com/adobe/da-admin/issues/45 for this.

Regarding 3., I'm not sure I follow - if you version again, it will override the previous version so there shouldn't be a new entry in the list. I suppose we could make it a new version and just have it be duplicated content but that would be something purely to accommodate the ability to create versions with a label. Alternatively, we could fail if you attempt to create a version without any change to the previous one (would be a boolean change in da-admin).

Created https://github.com/adobe/da-admin/issues/46 for this.