Overview

Proposed resolution

Create an api slice, publishAllPendingChanges that posts to the controller added in #3489994: Create an endpoint to publish all auto-saved entities.
Create a slice or extend an existing one to keep track of 'conflict ids'

The mutation can return a 409 when conflicts exist. In this case it will return the list of conflicted IDs and the new auto-save data. Use a manual cache update in the mutation to update the pending changes query data AND the conflict IDs

When conflict IDs exist, these will be marked up differently in the pending changes UI

The mutation will return a 200 on success, this should prompt the client to empty the pending changes and conflict IDs

User interface changes

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

effulgentsia created an issue. See original summary.

soaratul’s picture

What exact change we need to do in the UI for 👇?
When conflict IDs exist, these will be marked up differently in the pending changes UI

effulgentsia’s picture

I'd say for the scope of this issue, just make their text color red. We can defer a better design to a followup issue after this one is done. Better to get this one in sooner than hold it up on figuring out that design.

soaratul’s picture

I guess make text colour red for those that are not in the current list of redux store, but what about for the new items(I mean those are not in the redux store but are there in conflicts as additional item).

BTW it's very rare now we will get conflicts as clicking on Review N changes button now we are refetching pending changes also on every 10 seconds we are fetching.

larowlan’s picture

Title: [PP-1] Implement the "Publish All" button » Implement the "Publish All" button
Status: Postponed » Active

The blocker is in

wim leers’s picture

Assigned: Unassigned » soaratul
Priority: Normal » Critical
Status: Active » Needs work
larowlan’s picture

From a conversation in slack @soaratul noted there is no owner info in this

Blocker: Not sure how to proceed further? - let's discuss and decide.

{
    "errors": [
        {
            "code": 1,
            "detail": "This item is unexpected.",
            "source": {
                "pointer": "node:3:en"
            },
        },
        {
            "code": 2,
            "detail": "Expected item not present.",
            "source": {
                "pointer": "node:4:en"
            },
            "meta": {
                "entity_type": "node",
                "entity_id": "4",
                "label": "Drupal node Article"
            }
        }
    ]
}

I think what we need to do is include the auto-save key in meta so that you can match it against the records from the pending changes API

I'll open a separate issue for that

effulgentsia’s picture

Issue tags: +sprint

Up until now, the XB team has been following a pseudo-scrum/pseudo-kanban process, but we're now shifting into more conventional scrum. We started a new 2-week sprint last Thursday (Jan 16). I'm tagging our current sprint's issues for visibility.

lauriii’s picture

Assigned: soaratul » Unassigned
jessebaker’s picture

Assigned: Unassigned » jessebaker

Assigning to me to continue from @soaratul

jessebaker’s picture

Assigned: jessebaker » Unassigned
Status: Needs work » Needs review
jessebaker’s picture

Clicking publish all should publish changes. It currently doesn't, but it will at least show you an error as to why it can't publish.

There are follow ups

  1. As per @tedbow's suggestion in his review: polish handling for errors moved to follow up #3503404: [Needs design] Polish flow for Publish all changes" when receiving conflict errors in response
  2. Publishing currently fails on nodes due to an error with the URL alias field #3503347: Unable to create URL aliases with XB enabled due to incorrect validation constraint addition
  3. Changes show up immediately when previewing a node in XB #3502902: Only auto-save content entities/PageRegion config entities when there are actual changes: simply previewing incorrectly causes them to appear in "Review x changes"
  4. The pending changes API endpoint should list individual regions for global template changes. Currently adding content to a node will show two changes - one for the node and another for the template[#3500390]
wim leers’s picture

Title: Implement the "Publish All" button » [PP-1] Implement the "Publish All" button
Status: Needs review » Postponed

I did a review (and spotted a few nits) to try to answer #15 myself. AFAICT the answer is "yes".

If so, I propose to postpone this issue on #3503347: Unable to create URL aliases with XB enabled due to incorrect validation constraint addition and I'll prioritize tackling that on Monday.

wim leers’s picture

Title: [PP-1] Implement the "Publish All" button » Implement the "Publish All" button
Assigned: Unassigned » jessebaker
Status: Postponed » Needs work

Well … that was easier than I thought! @longwave did the work, I did the research to prove his fix was correct: #3503347: Unable to create URL aliases with XB enabled due to incorrect validation constraint addition is in!

jessebaker’s picture

Assigned: jessebaker » Unassigned
Status: Needs work » Needs review
hooroomoo’s picture

Assigned: Unassigned » hooroomoo

hooroomoo credited tedbow.

hooroomoo’s picture

  • hooroomoo committed 2b2f0e2e on 0.x authored by soaratul
    Issue #3497530 by jessebaker, soaratul, effulgentsia, wim leers,...
hooroomoo’s picture

Assigned: hooroomoo » Unassigned
Status: Needs review » Fixed

🎉

wim leers’s picture

Status: Fixed » Patch (to be ported)
Issue tags: +Needs screenshots

🤩

But can we get a short screencast to share this with the world? 🫣🙏

effulgentsia’s picture

Issue tags: -sprint
wim leers’s picture

Status: Patch (to be ported) » Fixed
Issue tags: -Needs screenshots

Not pragmatic to await this screenshot anymore, so marking fixed. This'll have to be demonstrated as part of other issues landing, then.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.