Default to current deck in csv import if settings allow it (#2527)

* Default to current deck in csv import if settings allow it

Reuses defaults_for_adding(). In the future we might also want to update
the last deck/notetype on successful completion, if entries weren't
specified in the file.

https://forums.ankiweb.net/t/importing-new-notes-to-wrong-deck-in-anki-2-1-63/30598

* Address review feedback from Rumo
This commit is contained in:
Damien Elmes 2023-05-31 13:47:12 +10:00 committed by GitHub
parent 7f6c410ca5
commit f758b33346
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 42 additions and 58 deletions

View file

@ -114,7 +114,8 @@ message CsvMetadataRequest {
string path = 1; string path = 1;
optional CsvMetadata.Delimiter delimiter = 2; optional CsvMetadata.Delimiter delimiter = 2;
optional int64 notetype_id = 3; optional int64 notetype_id = 3;
optional bool is_html = 4; optional int64 deck_id = 4;
optional bool is_html = 5;
} }
// Column indices are 1-based to make working with them in TS easier, where // Column indices are 1-based to make working with them in TS easier, where

View file

@ -50,8 +50,8 @@ impl Collection {
}) })
} }
/// The currently selected deck, the home deck of the provided card, or the /// The currently selected deck, the home deck of the provided card if
/// default deck. /// current deck is filtered, or the default deck.
fn get_current_deck_for_adding( fn get_current_deck_for_adding(
&mut self, &mut self,
home_deck_of_reviewer_card: DeckId, home_deck_of_reviewer_card: DeckId,

View file

@ -88,6 +88,7 @@ impl ImportExportService for Backend {
&input.path, &input.path,
delimiter, delimiter,
input.notetype_id.map(Into::into), input.notetype_id.map(Into::into),
input.deck_id.map(Into::into),
input.is_html, input.is_html,
) )
}) })

View file

@ -42,10 +42,11 @@ impl Collection {
path: &str, path: &str,
delimiter: Option<Delimiter>, delimiter: Option<Delimiter>,
notetype_id: Option<NotetypeId>, notetype_id: Option<NotetypeId>,
deck_id: Option<DeckId>,
is_html: Option<bool>, is_html: Option<bool>,
) -> Result<CsvMetadata> { ) -> Result<CsvMetadata> {
let mut reader = open_file(path)?; let mut reader = open_file(path)?;
self.get_reader_metadata(&mut reader, delimiter, notetype_id, is_html) self.get_reader_metadata(&mut reader, delimiter, notetype_id, deck_id, is_html)
} }
fn get_reader_metadata( fn get_reader_metadata(
@ -53,6 +54,7 @@ impl Collection {
mut reader: impl Read + Seek, mut reader: impl Read + Seek,
delimiter: Option<Delimiter>, delimiter: Option<Delimiter>,
notetype_id: Option<NotetypeId>, notetype_id: Option<NotetypeId>,
deck_id: Option<DeckId>,
is_html: Option<bool>, is_html: Option<bool>,
) -> Result<CsvMetadata> { ) -> Result<CsvMetadata> {
let mut metadata = CsvMetadata::from_config(self); let mut metadata = CsvMetadata::from_config(self);
@ -62,9 +64,8 @@ impl Collection {
maybe_set_fallback_is_html(&mut metadata, &records, is_html)?; maybe_set_fallback_is_html(&mut metadata, &records, is_html)?;
set_preview(&mut metadata, &records)?; set_preview(&mut metadata, &records)?;
maybe_set_fallback_columns(&mut metadata)?; maybe_set_fallback_columns(&mut metadata)?;
self.maybe_set_fallback_notetype(&mut metadata, notetype_id)?; self.maybe_set_notetype_and_deck(&mut metadata, notetype_id, deck_id)?;
self.maybe_init_notetype_map(&mut metadata)?; self.maybe_init_notetype_map(&mut metadata)?;
self.maybe_set_fallback_deck(&mut metadata)?;
Ok(metadata) Ok(metadata)
} }
@ -176,37 +177,33 @@ impl Collection {
} }
} }
fn maybe_set_fallback_notetype( /// Ensure notetype and deck are set.
///
/// - When the UI is first loaded, both notetype and deck arguments will be
/// None.
/// - When the UI refreshes due to user changes, the currently-selected deck
/// and notetype will be provided.
/// - Metadata may already have deck and notetype set, if those directives
/// were present in the file to import. In the UI refresh case, we
/// override them with the current UI values, so that the user can adjust
/// the deck/notetype if they wish.
/// - In the initial load case, if notetype/deck were not specified in file,
/// we apply the defaults from defaults_for_adding().
pub(crate) fn maybe_set_notetype_and_deck(
&mut self, &mut self,
metadata: &mut CsvMetadata, metadata: &mut crate::pb::import_export::CsvMetadata,
notetype_id: Option<NotetypeId>, notetype_id: Option<NotetypeId>,
deck_id: Option<DeckId>,
) -> Result<()> { ) -> Result<()> {
if let Some(ntid) = notetype_id { let defaults = self.defaults_for_adding(DeckId(0))?;
metadata.notetype = Some(CsvNotetype::new_global(ntid)); if metadata.notetype.is_none() || notetype_id.is_some() {
} else if metadata.notetype.is_none() { metadata.notetype = Some(CsvNotetype::new_global(
metadata.notetype = Some(CsvNotetype::new_global(self.fallback_notetype_id()?)); notetype_id.unwrap_or(defaults.notetype_id),
}
Ok(())
}
fn maybe_set_fallback_deck(&mut self, metadata: &mut CsvMetadata) -> Result<()> {
if metadata.deck.is_none() {
metadata.deck = Some(CsvDeck::DeckId(
metadata
.notetype_id()
.and_then(|ntid| self.default_deck_for_notetype(ntid).transpose())
.unwrap_or_else(|| {
self.get_current_deck().map(|deck| {
if deck.is_filtered() {
DeckId(1)
} else {
deck.id
}
})
})?
.0,
)); ));
} }
if metadata.deck.is_none() || deck_id.is_some() {
metadata.deck = Some(CsvDeck::DeckId(deck_id.unwrap_or(defaults.deck_id).0));
}
Ok(()) Ok(())
} }
@ -234,18 +231,6 @@ impl Collection {
} }
Ok(()) Ok(())
} }
fn fallback_notetype_id(&mut self) -> Result<NotetypeId> {
Ok(if let Some(notetype_id) = self.get_current_notetype_id() {
notetype_id
} else {
self.storage
.get_all_notetype_names()?
.first()
.or_invalid("collection has no notetypes")?
.0
})
}
} }
impl CsvMetadata { impl CsvMetadata {
@ -524,14 +509,6 @@ impl CsvNotetype {
} }
impl CsvMetadata { impl CsvMetadata {
fn notetype_id(&self) -> Option<NotetypeId> {
if let Some(CsvNotetype::GlobalNotetype(ref global)) = self.notetype {
Some(NotetypeId(global.id))
} else {
None
}
}
pub(super) fn meta_columns(&self) -> HashSet<usize> { pub(super) fn meta_columns(&self) -> HashSet<usize> {
let mut columns = HashSet::new(); let mut columns = HashSet::new();
if let Some(CsvDeck::DeckColumn(deck_column)) = self.deck { if let Some(CsvDeck::DeckColumn(deck_column)) = self.deck {
@ -579,7 +556,7 @@ mod test {
metadata!($col, $csv, None) metadata!($col, $csv, None)
}; };
($col:expr,$csv:expr, $delim:expr) => { ($col:expr,$csv:expr, $delim:expr) => {
$col.get_reader_metadata(Cursor::new($csv.as_bytes()), $delim, None, None) $col.get_reader_metadata(Cursor::new($csv.as_bytes()), $delim, None, None, None)
.unwrap() .unwrap()
}; };
} }

View file

@ -56,17 +56,20 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
deckColumn, deckColumn,
guidColumn, guidColumn,
); );
$: getCsvMetadata(path, delimiter, undefined, isHtml).then((meta) => { $: getCsvMetadata(path, delimiter, undefined, undefined, isHtml).then((meta) => {
columnLabels = meta.columnLabels; columnLabels = meta.columnLabels;
preview = meta.preview; preview = meta.preview;
}); });
$: if (globalNotetype?.id !== lastNotetypeId || delimiter !== lastDelimeter) { $: if (globalNotetype?.id !== lastNotetypeId || delimiter !== lastDelimeter) {
lastNotetypeId = globalNotetype?.id; lastNotetypeId = globalNotetype?.id;
lastDelimeter = delimiter; lastDelimeter = delimiter;
getCsvMetadata(path, delimiter, globalNotetype?.id).then((meta) => { getCsvMetadata(path, delimiter, globalNotetype?.id, deckId || undefined).then(
(meta) => {
globalNotetype = meta.globalNotetype ?? null; globalNotetype = meta.globalNotetype ?? null;
deckId = meta.deckId ?? null;
tagsColumn = meta.tagsColumn; tagsColumn = meta.tagsColumn;
}); },
);
} }
async function onImport(): Promise<void> { async function onImport(): Promise<void> {

View file

@ -60,6 +60,7 @@ export async function getCsvMetadata(
path: string, path: string,
delimiter?: ImportExport.CsvMetadata.Delimiter, delimiter?: ImportExport.CsvMetadata.Delimiter,
notetypeId?: number, notetypeId?: number,
deckId?: number,
isHtml?: boolean, isHtml?: boolean,
): Promise<ImportExport.CsvMetadata> { ): Promise<ImportExport.CsvMetadata> {
return importExport.getCsvMetadata( return importExport.getCsvMetadata(
@ -67,6 +68,7 @@ export async function getCsvMetadata(
path, path,
delimiter, delimiter,
notetypeId, notetypeId,
deckId,
isHtml, isHtml,
}), }),
); );