Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IBX-5222: Work with drafts #298

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

IBX-5222: Work with drafts #298

wants to merge 5 commits into from

Conversation

dabrt
Copy link
Contributor

@dabrt dabrt commented Jun 7, 2024

Question Answer
JIRA Ticket IBX-5222
Versions 4.6
Edition all

Describe mechanism of creating new drafts in draft conflict state

Preview
https://ez-systems-developer-documentation--298.com.readthedocs.build/projects/userguide/en/298/content_management/content_versions/#drafts

Checklist

  • Text renders correctly
  • Text has been checked with vale
  • Added link to this PR in relevant JIRA ticket or code PR

Copy link
Contributor

@juskora juskora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got the impression that changes don't explicitly explain the mechanism of draft conflicts, it also lacks good practices to avoid those conflicts.

docs/content_management/content_versions.md Outdated Show resolved Hide resolved
docs/content_management/content_versions.md Outdated Show resolved Hide resolved
Co-authored-by: Justyna Koralewicz <[email protected]>
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@dabrt dabrt requested a review from juskora June 11, 2024 14:07
@julitafalcondusza
Copy link
Contributor

“Describe mechanism of creating new drafts in draft conflict state” - wasn’t this the scope of the PR?

Copy link
Contributor

@julitafalcondusza julitafalcondusza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General comment: PR is a bit unclear and hard to review, as it contains two separate topics:

  1. Describe mechanism of creating new drafts in draft conflict state - scope of this PR
    and
  2. Reorganization and corrections of the remaining part - out of scope.

1 PR = 1 scope. I'd suggest to keep "Draft conflicts" part only in this PR.

Comment on lines +21 to +24
You then see a [**Draft conflict**](content_versions.md#draft-conflicts) screen, where you can pick a draft for editing, or create a new one.
The new draft is based on the published version of the content item and does not contain changes from other drafts.

![Draft conflict](img/draft_conflict.png "Draft conflict")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would move this to "Draft conflicts" section.
In my opinion, we shouldn't describe conflicts in another sections, like "Edit", because this shouldn't be treated as "normal" behaviour / good practice.

![All versions of a content item](img/content_item_versions.png "All versions of a content item")
#### Draft conflicts

If several drafts exist of the same content item, for example, created by different users or as a result of different processes, they can contain conflicting changes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rewrite it. As an opening sentence it should be shorter and really clear.
"Conflicting changes" - maybe you could rewrite it and use something like "can cause conflicts", etc?

#### Draft conflicts

If several drafts exist of the same content item, for example, created by different users or as a result of different processes, they can contain conflicting changes.
Such draft conflicts can be resolved by using the [Version Compare](work_with_versions.md#compare-versions) feature.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Version compare" - capital "C" not needed, this is what you can see when you hover over an icon:
version compare

Also, I'm not sure about calling it "Version compare" feature. It'd be more understandable if you say "you can compare versions".

## Compare versions
You can compare two versions of the same content item.
## Compare versions
You can compare two versions of the same content item, for example, to resolve a conflict between drafts coming from different sources.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above: I'd not add sentence about conflicts here.
In "Draft conflicts" section you can highlight comparison, it's enough.


![All versions of a content item](img/content_item_versions.png "All versions of a content item")
#### Draft conflicts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm missing here some more explanation, as @juskora mentioned in her comment.
You could show here how to resolve such conflict, etc.

Copy link
Contributor

@julitafalcondusza julitafalcondusza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see my comments.

Copy link
Contributor

@juskora juskora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, this PR needs to be rewritten with different approach to draft mechanisms explaining it's intricate nature.
Additionally, in this form, PR is hard to review as it's hard to spot what part were actually added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants