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

Add MART reports importer #5032

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

Add MART reports importer #5032

wants to merge 27 commits into from

Conversation

pacodelcastillolopez
Copy link
Collaborator

@pacodelcastillolopez pacodelcastillolopez commented Dec 18, 2024 •
edited by gjvoosten
Loading

Implementation of a MART reports importer and an admin GUI to monitor it.

Closes AB#1011

User changes

  • None

Superuser changes

  • None

Admin changes

  • Can see a log of the MART report importer in a new screen in the Admin area.

System admin changes

  • application.yml or anet-dictionary.yml needs change
    Update application.yml: 
        […]
    show-logout-link: true

  # Settings for the Exchange server to receive MART reports
  mart:
    hostname: <fqhn.of.mail.server>
    username: <…>
    password: <…>
    trusted-sender: <sender.of.mart.reports@mail.domain>
    disable-certificate-validation: false
    mark-as-read: true
    disabled: false
    mail-polling-delay-in-seconds: 10
    max-number-emails-pulled: 40

  # Set path of dictionary to be loaded
  […]
    and enable the MART GUI in the dictionary like so: 
    […]
regularUsersCanCreateLocations: true
engagementsIncludeTimeAndDuration: true
eventsIncludeStartAndEndTime: true
featureMartGuiEnabled: true
[…]
  • db needs migration
  • documentation has changed
  • graphql schema has changed

Checklist

  • described the user behavior in PR body
  • referenced/updated all related issues
  • commits follow a repo#issue: Title title format and these 7 rules
  • commits have a clean history, otherwise PR may be squash-merged
  • added and/or updated unit tests
  • added and/or updated e2e tests
  • added and/or updated data migrations
  • updated documentation
  • resolved all build errors and warnings
  • opened debt issues for anything not resolved here

@pacodelcastillolopez pacodelcastillolopez added improvement MART Mobile Advisor Reporting Tool labels Dec 18, 2024
@gjvoosten gjvoosten changed the title MART reports importer Add MART reports importer Dec 18, 2024
@gjvoosten gjvoosten marked this pull request as draft December 19, 2024 10:22
@gjvoosten gjvoosten force-pushed the AB#1011-be-able-to-import-MART-reports branch 8 times, most recently from 318d11c to 8890569 Compare January 7, 2025 10:52
@gjvoosten gjvoosten force-pushed the AB#1011-be-able-to-import-MART-reports branch 3 times, most recently from 50c3553 to a8ba7d0 Compare January 16, 2025 15:35
@gjvoosten gjvoosten force-pushed the AB#1011-be-able-to-import-MART-reports branch from a8ba7d0 to 91ba896 Compare January 20, 2025 16:30
@gjvoosten gjvoosten force-pushed the AB#1011-be-able-to-import-MART-reports branch 2 times, most recently from eb13870 to e47948d Compare February 3, 2025 13:59
@gjvoosten gjvoosten force-pushed the AB#1011-be-able-to-import-MART-reports branch from e47948d to a75589f Compare February 5, 2025 07:52
@gjvoosten
AB#1011 Add missing TRUNCATE
5412680
@gjvoosten
AB#1011 Remove unneeded GraphQL inputs
da61ec8
@gjvoosten
AB#1011 Improve Mart imported reports view
c62c4e8 
Show a nice icon indicating import success status.
Add missing avatar to person link.
Simplify code.
Remove unnecessary Component prop.
@gjvoosten
AB#1011 Filter out all MART email addresses
f921cbc
@gjvoosten
AB#1011 Use already existing asset
4cda41a
@gjvoosten
AB#1011 Improve MartImportedReportsResourceTest
7dca343
@gjvoosten
AB#1011 Simplify code
4973c93
@gjvoosten
AB#1011 Add missing foreign key constraint
14c1286 
Also use "boolean" type as everywhere else.
@gjvoosten
AB#1011 Delete unused ReadMailException
e180df8
@gjvoosten
AB#1011 Clean up code
49b516f
@gjvoosten
AB#1011 Correctly insert datetime
fa82cab
@gjvoosten
AB#1011 Add locationName to ReportDto
009eeee
@gjvoosten
AB#1011 Clean up after MartReportImporterWorkerTest
4a07aaf 
Delete imported reports, thus fixing MartImportedReportsResourceTest.
Properly report MART import errors as a list.
@gjvoosten
AB#1011 Make MART reports import GUI configurable through the dictionary
8211020
@gjvoosten
Work around SnakeYAML bug
42dfd08 
SnakeYAML 2.3 has a bug while reading a YAML file: when the last character
in the buffer is a multi-byte Unicode character, it may fail with:

  java.lang.ArrayIndexOutOfBoundsException: Index 1024 out of bounds for length 1024

It will be fixed in 2.4, but that hasn't been released yet, so simply
replace the Unicode character by some text.
@gjvoosten gjvoosten force-pushed the AB#1011-be-able-to-import-MART-reports branch from e699b63 to 42dfd08 Compare February 6, 2025 15:15
@gjvoosten gjvoosten marked this pull request as ready for review February 13, 2025 12:10
@gjvoosten gjvoosten requested review from midmarch and jlnat February 13, 2025 12:10
midmarch
midmarch approved these changes Feb 13, 2025
View reviewed changes
Copy link
Contributor

@midmarch midmarch left a comment

Choose a reason for hiding this comment

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

As discussed, we test in the Azure testbed after merging

gjvoosten
gjvoosten requested changes Feb 13, 2025
View reviewed changes
src/main/java/mil/dds/anet/threads/mart/MartReportImporterWorker.java
anetReport.setIntent(martReport.getIntent());
anetReport.setEngagementDate(martReport.getEngagementDate());
anetReport.setReportText(martReport.getReportText());
anetReport.setClassification("NKU");
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should not be hard-coded. Instead it should come from the report (there's a custom field called securityMarking that should contain the correct value). Site admins need to make sure that both sides have this marking in their configuration.

midmarch
midmarch requested changes Feb 13, 2025
View reviewed changes
Copy link
Contributor

@midmarch midmarch left a comment

Choose a reason for hiding this comment

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

I agree with @gjvoosten, we need to read the security marking preferably from the report, alternatively from the dictionary. But hard coding it to a value with is already configured in the dictionary, is not very solid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gjvoosten gjvoosten gjvoosten requested changes

@midmarch midmarch midmarch requested changes

@jlnat jlnat Awaiting requested review from jlnat

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
feature MART Mobile Advisor Reporting Tool
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pacodelcastillolopez @gjvoosten @midmarch