add import mode to update datasets without deleting existing data #308
Conversation
src/data/InstanceDhisRepository.ts
Outdated
| public async importDataPackage(dataPackage: DataPackage): Promise<SynchronizationResult[]> { | ||
| public async importDataPackage( | ||
| dataPackage: DataPackage, | ||
| createAndUpdate: boolean |
There was a problem hiding this comment.
Typically, it's better to use an options argument that wraps boolean options. This way, calls to the method are more declarative.
| return true; | ||
| } else { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Equivalent return strategy === "IMPORT"
| @@ -45,7 +40,7 @@ export default function ImportTemplatePage({ settings }: RouteComponentProps) { | |||
| const [orgUnitTreeFilter, setOrgUnitTreeFilter] = useState<string[]>([]); | |||
| const [importState, setImportState] = useState<ImportState>(); | |||
| const [messages, setMessages] = useState<string[]>([]); | |||
| const [dialogProps, updateDialog] = useState<ConfirmationDialogProps | null>(null); | |||
| const [dialogProps, updateDialog] = useState<ModalDialogProps | null>(null); | |||
There was a problem hiding this comment.
It's more idiomatic to use undefined as initial empty value of a useState. Then we can simply write: useState<ConfirmationDialogProps>();
| "IMPORT ONLY NEW DATA VALUES, WITHOUT UPDATING OR DELETING CURRENT ONES" | ||
| ), | ||
| updateTooltipText: i18n.t( | ||
| "IMPORT AND UPDATE DATA VALUES WITHOUT DELETING THE REMAINING EXISTING DATA" |
There was a problem hiding this comment.
Typically, we use standard case for texts (and the CSS decides how it's shown).
| switch (dataPackage.type) { | ||
| case "dataSets": { | ||
| const result = await this.importAggregatedData("CREATE_AND_UPDATE", dataPackage); | ||
| const result = await this.importAggregatedData( | ||
| createAndUpdate ? "CREATE_AND_UPDATE" : "CREATE", |
There was a problem hiding this comment.
[note] When Duplicate detection is not enabled, duplicateStrategy is "ERROR" so the default importStrategy is now CREATE instead of CREATE_AND_UPDATE. That's possibly a feature, not a bug, but I wanted to note it as it will change our default behavior for everyone.
eperedo
requested review from
Ramon-Jimenez and
tokland
and removed request for
saragilcas
There was a problem hiding this comment.
All good (functional testing). Waiting for feedback from @tokland before merging this.
|
@eperedo Is the base branch correct? or maybe development merge pending? |
eperedo
force-pushed
the
feat/datavalues-import-strategy
branch
from
d1e120e
to
e989446
Compare
📌 References
📝 Implementation
🔥 Notes for the reviewer
📹 Screenshots/Screen capture
Help Message
DELETE AND IMPORT help message (previous "PROCEED")
IMPORT ONLY NEW DATA VALUES help message
📑 Others