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.

Sign up for GitHub

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

Jump to bottom

[doc] Add shape on support to dynamically update representation descr… #4385

Merged
merged 2 commits into from
Jan 8, 2025
Merged

[doc] Add shape on support to dynamically update representation descr… #4385

merged 2 commits into from
Jan 8, 2025

Conversation

mcharfadi
Copy link
Contributor

@mcharfadi mcharfadi commented Jan 7, 2025 •
edited
Loading

…iption

Pull request template

General purpose

What is the main goal of this pull request?

  • Bug fixes
  • New features
  • Documentation
  • Cleanup
  • Tests
  • Build / releng

Project management

  • Has the pull request been added to the relevant project and milestone? (Only if you know that your work is part of a specific iteration such as the current one)
  • Have the priority: and pr: labels been added to the pull request? (In case of doubt, start with the labels priority: low and pr: to review later)
  • Have the relevant issues been added to the pull request?
  • Have the relevant labels been added to the issues? (area:, difficulty:, type:)
  • Have the relevant issues been added to the same project and milestone as the pull request?
  • Has the CHANGELOG.adoc been updated to reference the relevant issues?
  • Have the relevant API breaks been described in the CHANGELOG.adoc? (Including changes in the GraphQL API)
  • In case of a change with a visual impact, are there any screenshots in the CHANGELOG.adoc? For example in doc/screenshots/2022.5.0-my-new-feature.png

Architectural decision records (ADR)

  • Does the title of the commit contributing the ADR start with [doc]?
  • Are the ADRs mentioned in the relevant section of the CHANGELOG.adoc?

Dependencies

  • Are the new / upgraded dependencies mentioned in the relevant section of the CHANGELOG.adoc?
  • Are the new dependencies justified in the CHANGELOG.adoc?

Frontend

This section is not relevant if your contribution does not come with changes to the frontend.

General purpose

  • Is the code properly tested? (Plain old JavaScript tests for business code and tests based on React Testing Library for the components)

Typing

We need to improve the typing of our code, as such, we require every contribution to come with proper TypeScript typing for both changes contributing new files and those modifying existing files.
Please ensure that the following statements are true for each file created or modified (this may require you to improve code outside of your contribution).

  • Variables have a proper type
  • Functions’ arguments have a proper type
  • Functions’ return type are specified
  • Hooks are properly typed:
    • useMutation<DATA_TYPE, VARIABLE_TYPE>(…)
    • useQuery<DATA_TYPE, VARIABLE_TYPE>(…)
    • useSubscription<DATA_TYPE, VARIABLE_TYPE>(…)
    • useMachine<CONTEXT_TYPE, EVENTS_TYPE>(…)
    • useState<STATE_TYPE>(…)
  • All components have a proper typing for their props
  • No useless optional chaining with ?. (if the GraphQL API specifies that a field cannot be null, do not treat it has potentially null for example)
  • Nullable values have a proper type (for example let diagram: Diagram | null = null;)

Backend

This section is not relevant if your contribution does not come with changes to the backend.

General purpose

  • Are all the event handlers tested?
  • Are the event processor tested?
  • Is the business code (services) tested?
  • Are diagram layout changes tested?

Architecture

  • Are data structure classes properly separated from behavioral classes?
  • Are all the relevant fields final?
  • Is any data structure mutable? If so, please write a comment indicating why
  • Are behavioral classes either stateless or side effect free?

Review

How to test this PR?

Please describe here the various use cases to test this pull request

  • Has the Kiwi TCMS test suite been updated with tests for this contribution?

@mcharfadi mcharfadi added this to the 2025.2.0 milestone Jan 7, 2025
@mcharfadi mcharfadi requested a review from sbegaudeau as a code owner January 7, 2025 08:32
@mcharfadi mcharfadi self-assigned this Jan 7, 2025
@mcharfadi mcharfadi force-pushed the mch/doc/shape_update_representation_description branch from 44a338a to a719dc2 Compare January 7, 2025 09:47
@sbegaudeau sbegaudeau force-pushed the mch/doc/shape_update_representation_description branch from a719dc2 to 7557f0f Compare January 8, 2025 09:08
sbegaudeau
sbegaudeau approved these changes Jan 8, 2025
View reviewed changes
Copy link
Member

@sbegaudeau sbegaudeau left a comment

Choose a reason for hiding this comment

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

I've updated the PR with some clarifications. I'll merge it now but don't forget to have a look. Don't hesitate to tell me if something is not clear enough for you.

doc/iterations/2025.2/add_support_to_dynamically_update_ representation_description.adoc Outdated

This new studio will be a copy of the current description used by the representation and can be modified.

The representation can then be described by this new studio.
Copy link
Member

Choose a reason for hiding this comment

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

I will also add the acceptance criteria section which will be necessary for all shapes moving forward

doc/iterations/2025.2/add_support_to_dynamically_update_ representation_description.adoc Outdated

== Problem

A specifier can't update during runtime the description of a representation that was declared programmatically.
Copy link
Member

Choose a reason for hiding this comment

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

  • A specifier or an end user
  • We still only want to consider representation description created from the view DSL, programmatic representation descriptions are out of scope (I'll mention it as a no go)

doc/iterations/2025.2/add_support_to_dynamically_update_ representation_description.adoc Outdated
@@ -0,0 +1,45 @@
= Add support to dynamically update representation description
Copy link
Member

Choose a reason for hiding this comment

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

You don't need to add a blank line after each sentence of the whole document :)

doc/iterations/2025.2/add_support_to_dynamically_update_ representation_description.adoc Outdated

3 - He is redirected to a new studio where he can make changes to the representation (a modal was shown to validate the user choice before the redirection)

4 - When he is done, he clicks on the treeItemMenu associated with the description a new button : "publish description" in order to update the representation with the new description
Copy link
Member

Choose a reason for hiding this comment

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

I will remove this part since it is something that will be considered as part of another shape. Your work does not involve changing this. It should work just like a regular studio after that.

doc/iterations/2025.2/add_support_to_dynamically_update_ representation_description.adoc Outdated

1 - The page is refreshed and his representation is updated to the new description.

2 - He can still work with a representation not updated to the new description but the tree item corresponding to the representation shows a different icon or text color to notify him that an update is available, he can then click on the tree item menu "Update" to refresh the representation.
Copy link
Member

Choose a reason for hiding this comment

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

This would be way more complex than the budget that we have for this improvement. In this work we only want what is currently available for tables but for all representations, that's just it. We will make the whole mechanism more robust and generic but there won't be any major changes in behavior.

@sbegaudeau sbegaudeau merged commit 9cd3350 into master Jan 8, 2025
3 checks passed
@sbegaudeau sbegaudeau deleted the mch/doc/shape_update_representation_description branch January 8, 2025 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sbegaudeau sbegaudeau sbegaudeau approved these changes

Assignees

@mcharfadi mcharfadi

Labels
pr: to review soon priority: high
Projects
None yet
Milestone
2025.2.0
Development

Successfully merging this pull request may close these issues.
2 participants
@mcharfadi @sbegaudeau