Remove top_deck_id arg in deck_tree() (#1702)

Counts don't propogate correctly anymore (#1678).
This commit is contained in:
RumovZ 2022-03-02 06:30:32 +01:00 committed by GitHub
parent 88217c5e7d
commit 508b5ab947
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 25 additions and 68 deletions

View file

@ -126,7 +126,6 @@ message AddOrUpdateDeckLegacyRequest {
message DeckTreeRequest { message DeckTreeRequest {
// if non-zero, counts for the provided timestamp will be included // if non-zero, counts for the provided timestamp will be included
int64 now = 1; int64 now = 1;
int64 top_deck_id = 2;
} }
message DeckTreeNode { message DeckTreeNode {

View file

@ -174,7 +174,7 @@ class DeckManager(DeprecatedNamesMixin):
return deck return deck
def deck_tree(self) -> DeckTreeNode: def deck_tree(self) -> DeckTreeNode:
return self.col._backend.deck_tree(top_deck_id=0, now=0) return self.col._backend.deck_tree(now=0)
@classmethod @classmethod
def find_deck_in_tree( def find_deck_in_tree(

View file

@ -49,10 +49,13 @@ class SchedulerBase(DeprecatedNamesMixin):
# Deck list # Deck list
########################################################################## ##########################################################################
def deck_due_tree(self, top_deck_id: int = 0) -> DeckTreeNode: def deck_due_tree(self, top_deck_id: DeckId | None = None) -> DeckTreeNode | None:
"""Returns a tree of decks with counts. """Returns a tree of decks with counts.
If top_deck_id provided, counts are limited to that node.""" If top_deck_id provided, only the according subtree is returned."""
return self.col._backend.deck_tree(top_deck_id=top_deck_id, now=int_time()) tree = self.col._backend.deck_tree(now=int_time())
if top_deck_id:
return self.col.decks.find_deck_in_tree(tree, top_deck_id)
return tree
# Deck finished state & custom study # Deck finished state & custom study
########################################################################## ##########################################################################

View file

@ -85,8 +85,7 @@ class Scheduler(SchedulerBaseWithLegacy):
self._haveQueues = True self._haveQueues = True
def _reset_counts(self) -> None: def _reset_counts(self) -> None:
tree = self.deck_due_tree(self._current_deck_id) node = self.deck_due_tree(self._current_deck_id)
node = self.col.decks.find_deck_in_tree(tree, self._current_deck_id)
if not node: if not node:
# current deck points to a missing deck # current deck points to a missing deck
self.newCount = 0 self.newCount = 0

View file

@ -222,8 +222,7 @@ class Overview:
def _table(self) -> str | None: def _table(self) -> str | None:
counts = list(self.mw.col.sched.counts()) counts = list(self.mw.col.sched.counts())
current_did = self.mw.col.decks.get_current_id() current_did = self.mw.col.decks.get_current_id()
deck_tree = self.mw.col.sched.deck_due_tree(current_did) deck_node = self.mw.col.sched.deck_due_tree(current_did)
deck_node = self.mw.col.decks.find_deck_in_tree(deck_tree, current_did)
but = self.mw.button but = self.mw.button
buried_new = deck_node.new_count - counts[0] buried_new = deck_node.new_count - counts[0]

View file

@ -42,18 +42,13 @@ impl DecksService for Backend {
} }
fn deck_tree(&self, input: pb::DeckTreeRequest) -> Result<pb::DeckTreeNode> { fn deck_tree(&self, input: pb::DeckTreeRequest) -> Result<pb::DeckTreeNode> {
let lim = if input.top_deck_id > 0 {
Some(DeckId(input.top_deck_id))
} else {
None
};
self.with_col(|col| { self.with_col(|col| {
let now = if input.now == 0 { let now = if input.now == 0 {
None None
} else { } else {
Some(TimestampSecs(input.now)) Some(TimestampSecs(input.now))
}; };
col.deck_tree(now, lim) col.deck_tree(now)
}) })
} }

View file

@ -34,14 +34,9 @@ impl Collection {
&mut self, &mut self,
days_elapsed: u32, days_elapsed: u32,
learn_cutoff: u32, learn_cutoff: u32,
limit_to: Option<&str>,
) -> Result<HashMap<DeckId, DueCounts>> { ) -> Result<HashMap<DeckId, DueCounts>> {
self.storage.due_counts( self.storage
self.scheduler_version(), .due_counts(self.scheduler_version(), days_elapsed, learn_cutoff)
days_elapsed,
learn_cutoff,
limit_to,
)
} }
pub(crate) fn counts_for_deck_today( pub(crate) fn counts_for_deck_today(

View file

@ -287,14 +287,7 @@ impl Collection {
/// - Buried cards from previous days will be unburied if necessary. Because /// - Buried cards from previous days will be unburied if necessary. Because
/// this does not happen for future stamps, future due numbers may not be /// this does not happen for future stamps, future due numbers may not be
/// accurate. /// accurate.
/// - If top_deck_id is provided, only the node starting at the provided pub fn deck_tree(&mut self, timestamp: Option<TimestampSecs>) -> Result<DeckTreeNode> {
/// deck ID will have the counts populated. Currently the entire tree is
/// returned in this case, but this may change in the future.
pub fn deck_tree(
&mut self,
timestamp: Option<TimestampSecs>,
top_deck_id: Option<DeckId>,
) -> Result<DeckTreeNode> {
let names = self.storage.get_all_deck_names()?; let names = self.storage.get_all_deck_names()?;
let mut tree = deck_names_to_tree(names.into_iter()); let mut tree = deck_names_to_tree(names.into_iter());
@ -311,14 +304,12 @@ impl Collection {
let timing_today = self.timing_today()?; let timing_today = self.timing_today()?;
self.unbury_if_day_rolled_over(timing_today)?; self.unbury_if_day_rolled_over(timing_today)?;
let limit = top_deck_id
.and_then(|did| decks_map.get(&did).map(|deck| deck.name.as_native_str()));
let timing_at_stamp = self.timing_for_timestamp(timestamp)?; let timing_at_stamp = self.timing_for_timestamp(timestamp)?;
let days_elapsed = timing_at_stamp.days_elapsed; let days_elapsed = timing_at_stamp.days_elapsed;
let learn_cutoff = (timestamp.0 as u32) + self.learn_ahead_secs(); let learn_cutoff = (timestamp.0 as u32) + self.learn_ahead_secs();
let sched_ver = self.scheduler_version(); let sched_ver = self.scheduler_version();
let v3 = self.get_config_bool(BoolKey::Sched2021); let v3 = self.get_config_bool(BoolKey::Sched2021);
let counts = self.due_counts(days_elapsed, learn_cutoff, limit)?; let counts = self.due_counts(days_elapsed, learn_cutoff)?;
let dconf = self.storage.get_deck_config_map()?; let dconf = self.storage.get_deck_config_map()?;
add_counts(&mut tree, &counts); add_counts(&mut tree, &counts);
let limits = remaining_limits_map(decks_map.values(), &dconf, days_elapsed, v3); let limits = remaining_limits_map(decks_map.values(), &dconf, days_elapsed, v3);
@ -338,7 +329,7 @@ impl Collection {
pub fn current_deck_tree(&mut self) -> Result<Option<DeckTreeNode>> { pub fn current_deck_tree(&mut self) -> Result<Option<DeckTreeNode>> {
let target = self.get_current_deck_id(); let target = self.get_current_deck_id();
let tree = self.deck_tree(Some(TimestampSecs::now()), Some(target))?; let tree = self.deck_tree(Some(TimestampSecs::now()))?;
Ok(get_subnode(tree, target)) Ok(get_subnode(tree, target))
} }
@ -365,7 +356,7 @@ impl Collection {
impl Collection { impl Collection {
pub(crate) fn legacy_deck_tree(&mut self) -> Result<LegacyDueCounts> { pub(crate) fn legacy_deck_tree(&mut self) -> Result<LegacyDueCounts> {
let tree = self.deck_tree(Some(TimestampSecs::now()), None)?; let tree = self.deck_tree(Some(TimestampSecs::now()))?;
Ok(LegacyDueCounts::from(tree)) Ok(LegacyDueCounts::from(tree))
} }
@ -404,7 +395,7 @@ mod test {
col.get_or_create_normal_deck("2::c::A")?; col.get_or_create_normal_deck("2::c::A")?;
col.get_or_create_normal_deck("3")?; col.get_or_create_normal_deck("3")?;
let tree = col.deck_tree(None, None)?; let tree = col.deck_tree(None)?;
assert_eq!(tree.children.len(), 3); assert_eq!(tree.children.len(), 3);
@ -427,7 +418,7 @@ mod test {
col.storage.remove_deck(col.get_deck_id("2")?.unwrap())?; col.storage.remove_deck(col.get_deck_id("2")?.unwrap())?;
col.storage.remove_deck(col.get_deck_id("2::3")?.unwrap())?; col.storage.remove_deck(col.get_deck_id("2::3")?.unwrap())?;
let tree = col.deck_tree(None, None)?; let tree = col.deck_tree(None)?;
assert_eq!(tree.children.len(), 1); assert_eq!(tree.children.len(), 1);
Ok(()) Ok(())
@ -446,7 +437,7 @@ mod test {
note.set_field(0, "{{c1::}} {{c2::}} {{c3::}} {{c4::}}")?; note.set_field(0, "{{c1::}} {{c2::}} {{c3::}} {{c4::}}")?;
col.add_note(&mut note, child_deck.id)?; col.add_note(&mut note, child_deck.id)?;
let tree = col.deck_tree(Some(TimestampSecs::now()), None)?; let tree = col.deck_tree(Some(TimestampSecs::now()))?;
assert_eq!(tree.children[0].new_count, 4); assert_eq!(tree.children[0].new_count, 4);
assert_eq!(tree.children[0].children[0].new_count, 4); assert_eq!(tree.children[0].children[0].new_count, 4);
@ -457,7 +448,7 @@ mod test {
col.add_or_update_deck(&mut parent_deck)?; col.add_or_update_deck(&mut parent_deck)?;
// with the default limit of 20, there should still be 4 due // with the default limit of 20, there should still be 4 due
let tree = col.deck_tree(Some(TimestampSecs::now()), None)?; let tree = col.deck_tree(Some(TimestampSecs::now()))?;
assert_eq!(tree.children[0].new_count, 4); assert_eq!(tree.children[0].new_count, 4);
assert_eq!(tree.children[0].children[0].new_count, 4); assert_eq!(tree.children[0].children[0].new_count, 4);
@ -466,7 +457,7 @@ mod test {
conf.inner.new_per_day = 4; conf.inner.new_per_day = 4;
col.add_or_update_deck_config(&mut conf)?; col.add_or_update_deck_config(&mut conf)?;
let tree = col.deck_tree(Some(TimestampSecs::now()), None)?; let tree = col.deck_tree(Some(TimestampSecs::now()))?;
assert_eq!(tree.children[0].new_count, 3); assert_eq!(tree.children[0].new_count, 3);
assert_eq!(tree.children[0].children[0].new_count, 3); assert_eq!(tree.children[0].children[0].new_count, 3);
@ -505,7 +496,7 @@ mod test {
note.id.0 = 0; note.id.0 = 0;
col.add_note(&mut note, grandchild_2.id)?; col.add_note(&mut note, grandchild_2.id)?;
let parent = &col.deck_tree(Some(TimestampSecs::now()), None)?.children[0]; let parent = &col.deck_tree(Some(TimestampSecs::now()))?.children[0];
// grandchildren: own cards, limited by own new limits // grandchildren: own cards, limited by own new limits
assert_eq!(parent.children[0].children[0].new_count, 2); assert_eq!(parent.children[0].children[0].new_count, 2);
assert_eq!(parent.children[0].children[1].new_count, 1); assert_eq!(parent.children[0].children[1].new_count, 1);

View file

@ -579,7 +579,7 @@ mod test {
macro_rules! assert_counts { macro_rules! assert_counts {
($col:ident, $new:expr, $learn:expr, $review:expr) => {{ ($col:ident, $new:expr, $learn:expr, $review:expr) => {{
let tree = $col.deck_tree(Some(TimestampSecs::now()), None).unwrap(); let tree = $col.deck_tree(Some(TimestampSecs::now())).unwrap();
assert_eq!(tree.new_count, $new); assert_eq!(tree.new_count, $new);
assert_eq!(tree.learn_count, $learn); assert_eq!(tree.learn_count, $learn);
assert_eq!(tree.review_count, $review); assert_eq!(tree.review_count, $review);

View file

@ -265,10 +265,9 @@ impl SqliteStorage {
sched: SchedulerVersion, sched: SchedulerVersion,
day_cutoff: u32, day_cutoff: u32,
learn_cutoff: u32, learn_cutoff: u32,
top_deck: Option<&str>,
) -> Result<HashMap<DeckId, DueCounts>> { ) -> Result<HashMap<DeckId, DueCounts>> {
let sched_ver = sched as u8; let sched_ver = sched as u8;
let mut params = named_params! { let params = named_params! {
":new_queue": CardQueue::New as u8, ":new_queue": CardQueue::New as u8,
":review_queue": CardQueue::Review as u8, ":review_queue": CardQueue::Review as u8,
":day_cutoff": day_cutoff, ":day_cutoff": day_cutoff,
@ -279,30 +278,7 @@ impl SqliteStorage {
":preview_queue": CardQueue::PreviewRepeat as u8, ":preview_queue": CardQueue::PreviewRepeat as u8,
} }
.to_vec(); .to_vec();
let sql = concat!(include_str!("due_counts.sql"), " group by did");
let sql;
let prefix_start;
let prefix_end;
let top;
if let Some(top_inner) = top_deck {
// limited to deck node
top = top_inner;
prefix_start = format!("{}\x1f", top);
prefix_end = format!("{}\x20", top);
params.extend(named_params! {
":top_deck": top,
":prefix_start": prefix_start,
":prefix_end": prefix_end,
});
sql = concat!(
include_str!("due_counts.sql"),
" where did in (select id from decks where name = :top_deck ",
"or (name >= :prefix_start and name < :prefix_end)) group by did "
);
} else {
// entire tree
sql = concat!(include_str!("due_counts.sql"), " group by did");
}
self.db self.db
.prepare_cached(sql)? .prepare_cached(sql)?