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

[2.26.x] updated RTF transformer formatting and now omits null attributes from… #6744

Merged
merged 2 commits into from
Jun 9, 2023

Conversation

kcwire
Copy link
Member

@kcwire kcwire commented Jun 2, 2023

… output

What does this PR do?

Updates the RTF Transformers output formatting to include borders around the tables, removes the section breaks between each category, omits null valued attributes or the entire category if no attributes within a category have a value.

Who is reviewing it?

@glenhein
@jrnorth

How should this be tested?

Ingest a some data. Then export the results as RTF via the rest endpoint:
https://localhost:8993/services/catalog/query?q=*&format=rtf
Verify the formatting is updated and null valued attributes are not present.

Checklist:

  • Documentation Updated
  • Update / Add Threat Dragon models
  • [X ] Update / Add Unit Tests
  • Update / Add Integration Tests

Review Comment Legend:

  • ✏️ (Pencil) This comment is a nitpick or style suggestion, no action required for approval. This comment should provide a suggestion either as an in line code snippet or a gist.
  • ❓ (Question Mark) This comment is to gain a clearer understanding of design or code choices, clarification is required but action may not be necessary for approval.
  • ❗ (Exclamation Mark) This comment is critical and requires clarification or action before approval.

@@ -89,6 +89,7 @@ public void setAttributes(List<String> attributes) {
@Override
public Map<String, ExportValue> toExportMap(Metacard metacard) {
return attributes.stream()
.filter(s -> metacard.getAttribute(s) != null)
Copy link
Contributor

Choose a reason for hiding this comment

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

❓Do we want to remove attributes with null/empty values too?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. This avoids having multiple tables with no data in them as things like "Contact" are often empty.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just want to make sure we're on the same page - I'm asking if we need additional checks on the attribute's getValue() for null/empty.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, probably want to make sure it's not an empty string

@glenhein glenhein changed the title updated RTF transformer formatting and now omits null attributes from… [2.26.x] updated RTF transformer formatting and now omits null attributes from… Jun 6, 2023
@@ -89,6 +89,7 @@ public void setAttributes(List<String> attributes) {
@Override
public Map<String, ExportValue> toExportMap(Metacard metacard) {
return attributes.stream()
.filter(s -> metacard.getAttribute(s) != null)
Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, probably want to make sure it's not an empty string

@glenhein
Copy link
Contributor

glenhein commented Jun 7, 2023

I'm starting the hero

@glenhein
Copy link
Contributor

glenhein commented Jun 7, 2023

The RTF does look a lot better in MS Word. I don't like the right margin because it pushes to the edge of the paper. We probably need feedback between Leidos and @lifenouveau . I'l say the hero is ok for now, since it functions as advertised.

Screen Shot 2023-06-07 at 10 47 41 AM

@lifenouveau
Copy link

It might need a fixed width so the content would wrap instead of extend indefinitely.

@kcwire
Copy link
Member Author

kcwire commented Jun 8, 2023

The current library doesn't allow us to limit the width of the cells of a table. That may be doable but would likely require refactoring to use a different RTF library all together.

@lifenouveau
Copy link

The current library doesn't allow us to limit the width of the cells of a table. That may be doable but would likely require refactoring to use a different RTF library all together.

I'm assuming there isn't a way to force a wrap on long non-space strings like a URL?

@kcwire kcwire force-pushed the rtf-attr-filtering branch from 34d508b to 02ffdf7 Compare June 8, 2023 20:20
@kcwire
Copy link
Member Author

kcwire commented Jun 8, 2023

build now

@cxddfbot
Copy link

cxddfbot commented Jun 8, 2023

Internal build has been started, your results will be available at build completion.

@cxddfbot
Copy link

cxddfbot commented Jun 8, 2023

Build SUCCESS See the job results in legacy Jenkins UI or in Blue Ocean UI.

@jrnorth jrnorth merged commit 22d0f89 into 2.26.x Jun 9, 2023
alexabird added a commit that referenced this pull request Aug 8, 2024
alexabird added a commit that referenced this pull request Sep 13, 2024
alexabird added a commit that referenced this pull request Sep 13, 2024
malmgrens4 added a commit that referenced this pull request Nov 7, 2024
Forward Ports

Provide better transform failure message 390db3f
#6771

DDF-6386 Add support for source id and metacard type for csv metacard transforms
367e426
#6387

Adds Gmd QueryResponseTransformer
89877a4
#6781

Use UTC dates when exporting metacards in CSV format
27ed601
#6501

Fix CSV transformer output when no columnOrder given
fef3cb1
#6653

Updated the CsvTransformer to remove attributes that have empty or null
fbb2b4d
#6738

Xlsx column filtering
0335485
#6747

updated RTF transformer formatting and now omits null attributes from
64f8e5d
#6744

fix npe in rtf
41fe113
#6750

Updated CSV and XLSX transformers to maintain the order specified in the columnOrder argument
506791d
#6757

Dynamic rtf
83bcf56
#6762

Fix multi-value exports for RTF
dd7bc91
#6767

---------

Co-authored-by: derekwilhelm <[email protected]>
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.

5 participants