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

Common.DraftXxx: sync with RAP implementation #192

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

ralfhandl
Copy link
Member

@ralfhandl ralfhandl commented Jul 20, 2022

  1. DiscardAction is no longer experimental
  2. AdditionalNewActions is no longer experimental
  3. EditAction has a new optional parameter „PreserveChanges“ (Edm.Boolean)
  • The action can provide an optional parameter “PreserveChanges” of type “Edm.Boolean” that may be used to ensure outdated drafts of another user are not overwritten (see below).
  1. NewAction and AdditionalNewActions have an optional parameter “ResultIsActiveEntity” (Edm.Boolean)
  • The action can provide an optional parameter "ResultIsActiveEntity" of type Edm.Boolean to indicate whether via the action a draft or an active instance should be created. Default is “false”, i.e. the creation of a draft instance.
  1. DraftNode: PreparationAction is deprecated
  • @RaizoC wonders if the complete annotation “DraftNode” is deprecated as it is always empty (in the meantime). That a node is activated via and belongs to the draft root is indicated via a separate annotation DraftActivationVia. @MarcelWaechter: can you confirm the annotation “DraftNode” is NOT used any more in FE V4 (and V2)?
  1. ShareAction has a new parameter “ShareAll” (Edm.Boolean)

RaizoC
RaizoC previously approved these changes Jul 25, 2022
@RaizoC RaizoC self-requested a review July 25, 2022 14:09
Copy link
Contributor

@RaizoC RaizoC left a comment

Choose a reason for hiding this comment

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

To be consistent may the following sentence should also be changed: "New drafts may also be created by POSTing an empty entity without any properties to the entity set."

"New drafts may also be created by POSTing an entity with property IsActiveEntity=false (default) to the entity set."

In fact also during the creation of a draft root already properties may be set or even have to be set if the "requiredProperties" annotation is used. Therefore the statement with an "empty entity" is not correct.

Copy link
Contributor

@HeikoTheissen HeikoTheissen left a comment

Choose a reason for hiding this comment

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

Purely typographical review

vocabularies/Common.xml Outdated Show resolved Hide resolved
vocabularies/Common.xml Outdated Show resolved Hide resolved
vocabularies/Common.xml Outdated Show resolved Hide resolved
vocabularies/Common.xml Outdated Show resolved Hide resolved
vocabularies/Common.xml Outdated Show resolved Hide resolved
vocabularies/Common.xml Outdated Show resolved Hide resolved
vocabularies/Common.xml Outdated Show resolved Hide resolved
@RaizoC
Copy link
Contributor

RaizoC commented Nov 29, 2024

If we do not deprecate the complete "DraftNode" (which is only used as tag and thus redundant to "DraftActivationVia", we should at least remove or deprecate the ValidationFunction inside that was never used.

@HeikoTheissen HeikoTheissen dismissed their stale review November 29, 2024 11:01

Typographically OK

@ralfhandl
Copy link
Member Author

ralfhandl commented Nov 29, 2024

we should at least remove or deprecate the ValidationFunction

That is already deprecated:

<Property Name="ValidationFunction" Type="Common.QualifiedName">
<Annotation Term="Core.Revisions">
<Collection>
<Record>
<PropertyValue Property="Kind" EnumMember="Core.RevisionKind/Deprecated" />
<PropertyValue Property="Description" String="Separate validation without side-effects is not useful" />
</Record>
</Collection>
</Annotation>
<Annotation Term="Core.Description" String="Function that validates whether a draft document is ready for activation" />
</Property>

@RaizoC Should we deprecate the whole DraftNode now?

@ralfhandl
Copy link
Member Author

@RaizoC Should we deprecate the whole DraftNode now?

Deprecated the term

@RaizoC
Copy link
Contributor

RaizoC commented Dec 3, 2024

@RaizoC Should we deprecate the whole DraftNode now?

Deprecated the term

I would deprecate it as it only adds redundancy, but we @MarcelWaechter should confirm.

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

Successfully merging this pull request may close these issues.

3 participants