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

Add support fetching description entity #735

Merged
merged 14 commits into from
May 8, 2023
Merged

Conversation

pingsutw
Copy link
Member

@pingsutw pingsutw commented Mar 28, 2023

TL;DR

  • Fetch / list description entity
  • Add short description to SearchableTaskNameList
  • Add short description to SearchableWorkflowNameList
  • Add long description and source link to EntityDetails

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

image

image

image

Tracking Issue

NA

Follow-up issue

NA

Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
@pingsutw pingsutw marked this pull request as draft March 28, 2023 19:50
@codecov
Copy link

codecov bot commented Mar 28, 2023

Codecov Report

Merging #735 (5502df9) into master (e95faa8) will increase coverage by 0.02%.
The diff coverage is 83.67%.

@@            Coverage Diff             @@
##           master     #735      +/-   ##
==========================================
+ Coverage   66.06%   66.09%   +0.02%     
==========================================
  Files         485      488       +3     
  Lines       11764    11806      +42     
  Branches     2164     2171       +7     
==========================================
+ Hits         7772     7803      +31     
- Misses       3992     4003      +11     
Impacted Files Coverage Δ
...ackages/console/src/components/Entities/strings.ts 100.00% <ø> (ø)
...ole/src/components/Workflow/useWorkflowInfoItem.ts 53.65% <0.00%> (-1.35%) ⬇️
packages/console/src/models/Common/constants.ts 100.00% <ø> (ø)
...ges/console/src/components/hooks/useDescription.ts 70.00% <70.00%> (ø)
...sole/src/components/Entities/EntityDescription.tsx 81.13% <85.71%> (-1.10%) ⬇️
...ckages/console/src/models/DescriptionEntity/api.ts 87.50% <87.50%> (ø)
...onsole/src/components/Task/SimpleTaskInterface.tsx 100.00% <100.00%> (ø)
...components/Workflow/SearchableWorkflowNameList.tsx 74.77% <100.00%> (+0.46%) ⬆️
packages/console/src/components/data/apiContext.ts 92.85% <100.00%> (+0.54%) ⬆️
packages/console/src/models/Common/utils.ts 88.88% <100.00%> (+4.27%) ⬆️
... and 1 more

... and 2 files with indirect coverage changes

Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
@pingsutw pingsutw changed the title [WIP] Add support fetching description entity Add support fetching description entity Mar 29, 2023
@pingsutw pingsutw marked this pull request as ready for review March 29, 2023 18:55
className={classnames({
[commonStyles.hintText]: !hasDescription,
})}
>
{hasDescription
? metadata.description
? descriptionEntity?.longDescription?.value
: t(
patternKey('noDescription', entityStrings[id.resourceType]),
Copy link
Contributor

Choose a reason for hiding this comment

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

For some reason this bit doesn't seem to be working; ran a debugger and verified hasDescription is false but it doesn't seem to render the string?

Screenshot 2023-03-30 at 1 03 04 PM

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed it.

image

image

Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
@pradithya
Copy link
Member

It's a neat feature. Is there any blocker for merging it? @pingsutw @jsonporter

@jsonporter jsonporter merged commit e28a142 into flyteorg:master May 8, 2023
11 checks passed
ursucarina pushed a commit that referenced this pull request May 10, 2023
* Add Description entity

Signed-off-by: Kevin Su <pingsutw@apache.org>

* Add Description entity

Signed-off-by: Kevin Su <pingsutw@apache.org>

* nit

Signed-off-by: Kevin Su <pingsutw@apache.org>

* nit

Signed-off-by: Kevin Su <pingsutw@apache.org>

* nit

Signed-off-by: Kevin Su <pingsutw@apache.org>

* address comment

Signed-off-by: Kevin Su <pingsutw@apache.org>

* test ci

Signed-off-by: Kevin Su <pingsutw@apache.org>

* test ci

Signed-off-by: Kevin Su <pingsutw@apache.org>

* test ci

Signed-off-by: Kevin Su <pingsutw@apache.org>

* test ci

Signed-off-by: Kevin Su <pingsutw@apache.org>

* test ci

Signed-off-by: Kevin Su <pingsutw@apache.org>

---------

Signed-off-by: Kevin Su <pingsutw@apache.org>
Co-authored-by: Jason Porter <84735036+jsonporter@users.noreply.github.com>
Signed-off-by: Carina Ursu <carina@union.ai>
@flyte-bot
Copy link
Collaborator

🎉 This PR is included in version 1.7.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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