Global new ignore review limit (#2417)

* Add CardAdder test helper

* Add option to have new cards ignore the review limit

Also entails a lot of refactoring because the old code was deeply
coupled to the previous behaviour.

* Add global option to ignore review limit

* Refactor decrementation

* Unify testing
This commit is contained in:
RumovZ 2023-03-06 10:06:12 +01:00 committed by GitHub
parent a84d699271
commit bb297b95bc
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 470 additions and 254 deletions

View file

@ -34,11 +34,17 @@ deck-config-limit-new-bound-by-reviews =
shown.
deck-config-limit-interday-bound-by-reviews =
The review limit also affects interday learning cards. When applying the limit,
interday learning cards are fetched first, then reviews, and finally new cards.
interday learning cards are fetched first, then reviews.
deck-config-tab-description =
- `Preset`: The limit is shared with all decks using this preset.
- `This deck`: The limit is specific to this deck.
- `Today only`: Make a temporary change to this deck's limit.
deck-config-new-cards-ignore-review-limit = New cards ignore review limit
deck-config-new-cards-ignore-review-limit-tooltip =
By default, the review limit also applies to new cards, and no new cards will be
shown when the review limit has been reached. If this option is enabled, new cards
will be shown regardless of the review limit.
deck-config-affects-entire-collection = Affects the entire collection.
## Daily limit tabs: please try to keep these as short as the English version,
## as longer text will not fit on small screens.

View file

@ -172,6 +172,8 @@ message DeckConfigsForUpdate {
bool v3_scheduler = 5;
// only applies to v3 scheduler
string card_state_customizer = 6;
// only applies to v3 scheduler
bool new_cards_ignore_review_limit = 7;
}
message UpdateDeckConfigsRequest {
@ -183,4 +185,5 @@ message UpdateDeckConfigsRequest {
bool apply_to_children = 4;
string card_state_customizer = 5;
DeckConfigsForUpdate.CurrentDeck.Limits limits = 6;
bool new_cards_ignore_review_limit = 7;
}

View file

@ -108,6 +108,7 @@ impl From<pb::deckconfig::UpdateDeckConfigsRequest> for UpdateDeckConfigsRequest
apply_to_children: c.apply_to_children,
card_state_customizer: c.card_state_customizer,
limits: c.limits.unwrap_or_default(),
new_cards_ignore_review_limit: c.new_cards_ignore_review_limit,
}
}
}

View file

@ -21,17 +21,18 @@ pub enum BoolKey {
CollapseToday,
FutureDueShowBacklog,
HideAudioPlayButtons,
IgnoreAccentsInSearch,
InterruptAudioWhenAnswering,
NewCardsIgnoreReviewLimit,
PasteImagesAsPng,
PasteStripsFormatting,
PreviewBothSides,
Sched2021,
IgnoreAccentsInSearch,
RestorePositionBrowser,
RestorePositionReviewer,
ResetCountsBrowser,
ResetCountsReviewer,
RandomOrderReposition,
Sched2021,
ShiftPositionOfExistingCards,
#[strum(to_string = "normalize_note_text")]

View file

@ -27,6 +27,7 @@ pub struct UpdateDeckConfigsRequest {
pub apply_to_children: bool,
pub card_state_customizer: String,
pub limits: Limits,
pub new_cards_ignore_review_limit: bool,
}
impl Collection {
@ -45,6 +46,7 @@ impl Collection {
.schema_changed_since_sync(),
v3_scheduler: self.get_config_bool(BoolKey::Sched2021),
card_state_customizer: self.get_config_string(StringKey::CardStateCustomizer),
new_cards_ignore_review_limit: self.get_config_bool(BoolKey::NewCardsIgnoreReviewLimit),
})
}
@ -191,13 +193,17 @@ impl Collection {
}
self.set_config_string_inner(StringKey::CardStateCustomizer, &input.card_state_customizer)?;
self.set_config_bool_inner(
BoolKey::NewCardsIgnoreReviewLimit,
input.new_cards_ignore_review_limit,
)?;
Ok(())
}
/// Adjust the remaining steps of cards in the given deck according to the
/// config change.
fn adjust_remaining_steps_in_deck(
pub(crate) fn adjust_remaining_steps_in_deck(
&mut self,
deck: DeckId,
previous_config: Option<&DeckConfig>,
@ -286,8 +292,9 @@ mod test {
col.add_note(&mut note, DeckId(1))?;
}
// add the key so it doesn't trigger a change below
// add the keys so it doesn't trigger a change below
col.set_config_string_inner(StringKey::CardStateCustomizer, "")?;
col.set_config_bool_inner(BoolKey::NewCardsIgnoreReviewLimit, false)?;
// pretend we're in sync
let stamps = col.storage.get_collection_timestamps()?;
@ -320,6 +327,7 @@ mod test {
apply_to_children: false,
card_state_customizer: "".to_string(),
limits: Limits::default(),
new_cards_ignore_review_limit: false,
};
assert!(!col.update_deck_configs(input.clone())?.changes.had_change());

View file

@ -16,6 +16,12 @@ use crate::deckconfig::DeckConfigId;
use crate::pb::decks::deck::normal::DayLimit;
use crate::prelude::*;
#[derive(Debug, Clone, Copy)]
pub(crate) enum LimitKind {
Review,
New,
}
impl NormalDeck {
/// The deck's review limit for today, or its regular one, if any is
/// configured.
@ -50,15 +56,29 @@ impl DayLimit {
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub(crate) struct RemainingLimits {
pub review: u32,
pub new: u32,
pub(crate) review: u32,
pub(crate) new: u32,
pub(crate) cap_new_to_review: bool,
}
impl RemainingLimits {
pub(crate) fn new(deck: &Deck, config: Option<&DeckConfig>, today: u32, v3: bool) -> Self {
pub(crate) fn new(
deck: &Deck,
config: Option<&DeckConfig>,
today: u32,
v3: bool,
new_cards_ignore_review_limit: bool,
) -> Self {
if let Ok(normal) = deck.normal() {
if let Some(config) = config {
return Self::new_for_normal_deck(deck, today, v3, normal, config);
return Self::new_for_normal_deck(
deck,
today,
v3,
new_cards_ignore_review_limit,
normal,
config,
);
}
}
Self::default()
@ -68,29 +88,62 @@ impl RemainingLimits {
deck: &Deck,
today: u32,
v3: bool,
new_cards_ignore_review_limit: bool,
normal: &NormalDeck,
config: &DeckConfig,
) -> RemainingLimits {
let (review_limit, new_limit) = if v3 {
let review_limit = normal
.current_review_limit(today)
.unwrap_or(config.inner.reviews_per_day);
let new_limit = normal
.current_new_limit(today)
.unwrap_or(config.inner.new_per_day);
(review_limit, new_limit)
} else {
(config.inner.reviews_per_day, config.inner.new_per_day)
};
let (new_today, mut rev_today) = deck.new_rev_counts(today);
if v3 {
// any reviewed new cards contribute to the review limit
rev_today += new_today;
Self::new_for_normal_deck_v3(deck, today, new_cards_ignore_review_limit, normal, config)
} else {
Self::new_for_normal_deck_v2(deck, today, config)
}
}
fn new_for_normal_deck_v2(deck: &Deck, today: u32, config: &DeckConfig) -> RemainingLimits {
let review_limit = config.inner.reviews_per_day;
let new_limit = config.inner.new_per_day;
let (new_today_count, review_today_count) = deck.new_rev_counts(today);
Self {
review: (review_limit as i32 - review_today_count).max(0) as u32,
new: (new_limit as i32 - new_today_count).max(0) as u32,
cap_new_to_review: false,
}
}
fn new_for_normal_deck_v3(
deck: &Deck,
today: u32,
new_cards_ignore_review_limit: bool,
normal: &NormalDeck,
config: &DeckConfig,
) -> RemainingLimits {
let mut review_limit = normal
.current_review_limit(today)
.unwrap_or(config.inner.reviews_per_day) as i32;
let mut new_limit = normal
.current_new_limit(today)
.unwrap_or(config.inner.new_per_day) as i32;
let (new_today_count, review_today_count) = deck.new_rev_counts(today);
review_limit -= review_today_count;
new_limit -= new_today_count;
if !new_cards_ignore_review_limit {
review_limit -= new_today_count;
new_limit = new_limit.min(review_limit);
}
Self {
review: (review_limit as i32 - rev_today).max(0) as u32,
new: (new_limit as i32 - new_today).max(0) as u32,
review: review_limit.max(0) as u32,
new: new_limit.max(0) as u32,
cap_new_to_review: !new_cards_ignore_review_limit,
}
}
pub(crate) fn get(&self, kind: LimitKind) -> u32 {
match kind {
LimitKind::Review => self.review,
LimitKind::New => self.new,
}
}
@ -98,6 +151,31 @@ impl RemainingLimits {
self.review = self.review.min(limits.review);
self.new = self.new.min(limits.new);
}
/// True if some limit was decremented to 0.
fn decrement(&mut self, kind: LimitKind) -> DecrementResult {
let before = *self;
if matches!(kind, LimitKind::Review) {
self.review = self.review.saturating_sub(1);
}
if self.cap_new_to_review || matches!(kind, LimitKind::New) {
self.new = self.new.saturating_sub(1);
}
DecrementResult::new(&before, self)
}
}
struct DecrementResult {
count_reached_zero: bool,
}
impl DecrementResult {
fn new(before: &RemainingLimits, after: &RemainingLimits) -> Self {
Self {
count_reached_zero: before.review > 0 && after.review == 0
|| before.new > 0 && after.new == 0,
}
}
}
impl Default for RemainingLimits {
@ -105,6 +183,7 @@ impl Default for RemainingLimits {
RemainingLimits {
review: 9999,
new: 9999,
cap_new_to_review: false,
}
}
}
@ -114,6 +193,7 @@ pub(crate) fn remaining_limits_map<'a>(
config: &'a HashMap<DeckConfigId, DeckConfig>,
today: u32,
v3: bool,
new_cards_ignore_review_limit: bool,
) -> HashMap<DeckId, RemainingLimits> {
decks
.map(|deck| {
@ -124,6 +204,7 @@ pub(crate) fn remaining_limits_map<'a>(
deck.config_id().and_then(|id| config.get(&id)),
today,
v3,
new_cards_ignore_review_limit,
),
)
})
@ -140,7 +221,12 @@ struct NodeLimits {
}
impl NodeLimits {
fn new(deck: &Deck, config: &HashMap<DeckConfigId, DeckConfig>, today: u32) -> Self {
fn new(
deck: &Deck,
config: &HashMap<DeckConfigId, DeckConfig>,
today: u32,
new_cards_ignore_review_limit: bool,
) -> Self {
Self {
deck_id: deck.id,
level: deck.name.components().count(),
@ -149,6 +235,7 @@ impl NodeLimits {
deck.config_id().and_then(|id| config.get(&id)),
today,
true,
new_cards_ignore_review_limit,
),
}
}
@ -162,8 +249,7 @@ pub(crate) struct LimitTreeMap {
// and (3) have more than 1 tree, it's safe to unwrap on Tree::get() and
// Tree::root_node_id(), even if we clone Nodes.
tree: Tree<NodeLimits>,
/// A map to access the tree node of a deck. Only decks with a remaining
/// limit above zero are included.
/// A map to access the tree node of a deck.
map: HashMap<DeckId, NodeId>,
}
@ -174,21 +260,26 @@ impl LimitTreeMap {
child_decks: Vec<Deck>,
config: &HashMap<DeckConfigId, DeckConfig>,
today: u32,
new_cards_ignore_review_limit: bool,
) -> Self {
let root_limits = NodeLimits::new(root_deck, config, today);
let root_limits = NodeLimits::new(root_deck, config, today, new_cards_ignore_review_limit);
let mut tree = Tree::new();
let root_id = tree
.insert(Node::new(root_limits), InsertBehavior::AsRoot)
.unwrap();
let mut map = HashMap::new();
if root_limits.limits.review > 0 {
map.insert(root_deck.id, root_id.clone());
}
map.insert(root_deck.id, root_id.clone());
let mut limits = Self { tree, map };
let mut remaining_decks = child_decks.into_iter().peekable();
limits.add_child_nodes(root_id, &mut remaining_decks, config, today);
limits.add_child_nodes(
root_id,
&mut remaining_decks,
config,
today,
new_cards_ignore_review_limit,
);
limits
}
@ -204,6 +295,7 @@ impl LimitTreeMap {
remaining_decks: &mut Peekable<impl Iterator<Item = Deck>>,
config: &HashMap<DeckConfigId, DeckConfig>,
today: u32,
new_cards_ignore_review_limit: bool,
) {
let parent = *self.tree.get(&parent_node_id).unwrap().data();
while let Some(deck) = remaining_decks.peek() {
@ -214,7 +306,13 @@ impl LimitTreeMap {
}
l if l == parent.level + 1 => {
// next item is an immediate descendent of parent
self.insert_child_node(deck, parent_node_id.clone(), config, today);
self.insert_child_node(
deck,
parent_node_id.clone(),
config,
today,
new_cards_ignore_review_limit,
);
remaining_decks.next();
}
_ => {
@ -227,7 +325,13 @@ impl LimitTreeMap {
.last()
.cloned()
{
self.add_child_nodes(last_child_node_id, remaining_decks, config, today)
self.add_child_nodes(
last_child_node_id,
remaining_decks,
config,
today,
new_cards_ignore_review_limit,
)
} else {
// immediate parent is missing, skip the deck until a DB check is run
remaining_decks.next();
@ -243,12 +347,13 @@ impl LimitTreeMap {
parent_node_id: NodeId,
config: &HashMap<DeckConfigId, DeckConfig>,
today: u32,
new_cards_ignore_review_limit: bool,
) {
let mut child_limits = NodeLimits::new(child_deck, config, today);
let mut child_limits =
NodeLimits::new(child_deck, config, today, new_cards_ignore_review_limit);
child_limits
.limits
.cap_to(self.tree.get(&parent_node_id).unwrap().data().limits);
.cap_to(self.get_node_limits(&parent_node_id));
let child_node_id = self
.tree
.insert(
@ -256,16 +361,34 @@ impl LimitTreeMap {
InsertBehavior::UnderNode(&parent_node_id),
)
.unwrap();
if child_limits.limits.review > 0 {
self.map.insert(child_deck.id, child_node_id);
}
}
pub(crate) fn root_limit_reached(&self) -> bool {
self.map.is_empty()
self.map.insert(child_deck.id, child_node_id);
}
pub(crate) fn limit_reached(&self, deck_id: DeckId) -> bool {
self.map.get(&deck_id).is_none()
fn get_node_id(&self, deck_id: DeckId) -> Result<&NodeId> {
self.map
.get(&deck_id)
.or_invalid("deck not found in limits map")
}
fn get_node_limits(&self, node_id: &NodeId) -> RemainingLimits {
self.tree.get(node_id).unwrap().data().limits
}
fn get_deck_limits(&self, deck_id: DeckId) -> Result<RemainingLimits> {
self.get_node_id(deck_id)
.map(|node_id| self.get_node_limits(node_id))
}
fn get_root_limits(&self) -> RemainingLimits {
self.get_node_limits(self.tree.root_node_id().unwrap())
}
pub(crate) fn root_limit_reached(&self, kind: LimitKind) -> bool {
self.get_root_limits().get(kind) == 0
}
pub(crate) fn limit_reached(&self, deck_id: DeckId, kind: LimitKind) -> Result<bool> {
Ok(self.get_deck_limits(deck_id)?.get(kind) == 0)
}
pub(crate) fn active_decks(&self) -> Vec<DeckId> {
@ -276,59 +399,36 @@ impl LimitTreeMap {
.collect()
}
pub(crate) fn remaining_node_id(&self, deck_id: DeckId) -> Option<NodeId> {
self.map.get(&deck_id).map(Clone::clone)
pub(crate) fn decrement_deck_and_parent_limits(
&mut self,
deck_id: DeckId,
kind: LimitKind,
) -> Result<()> {
let node_id = self.get_node_id(deck_id)?.clone();
self.decrement_node_and_parent_limits(&node_id, kind);
Ok(())
}
pub(crate) fn decrement_node_and_parent_limits(&mut self, node_id: &NodeId, new: bool) {
fn decrement_node_and_parent_limits(&mut self, node_id: &NodeId, kind: LimitKind) {
let node = self.tree.get_mut(node_id).unwrap();
let parent = node.parent().cloned();
let limit = &mut node.data_mut().limits;
if if new {
limit.new = limit.new.saturating_sub(1);
limit.new
} else {
limit.review = limit.review.saturating_sub(1);
limit.review
} == 0
{
self.remove_node_and_descendants_from_map(node_id);
let limits = &mut node.data_mut().limits;
if limits.decrement(kind).count_reached_zero {
let limits = *limits;
self.cap_node_and_descendants(node_id, limits);
};
if let Some(parent_id) = parent {
self.decrement_node_and_parent_limits(&parent_id, new)
self.decrement_node_and_parent_limits(&parent_id, kind)
}
}
pub(crate) fn remove_node_and_descendants_from_map(&mut self, node_id: &NodeId) {
let node = self.tree.get(node_id).unwrap();
self.map.remove(&node.data().deck_id);
for child_id in node.children().clone() {
self.remove_node_and_descendants_from_map(&child_id);
}
}
pub(crate) fn cap_new_to_review(&mut self) {
self.cap_new_to_review_rec(&self.tree.root_node_id().unwrap().clone(), 9999);
}
fn cap_new_to_review_rec(&mut self, node_id: &NodeId, parent_limit: u32) {
fn cap_node_and_descendants(&mut self, node_id: &NodeId, limits: RemainingLimits) {
let node = self.tree.get_mut(node_id).unwrap();
let mut limits = &mut node.data_mut().limits;
limits.new = limits.new.min(limits.review).min(parent_limit);
// clone because of borrowing rules
let node_limit = limits.new;
let children = node.children().clone();
if node_limit == 0 {
self.remove_node_and_descendants_from_map(node_id);
}
for child_id in children {
self.cap_new_to_review_rec(&child_id, node_limit);
node.data_mut().limits.cap_to(limits);
for child_id in node.children().clone() {
self.cap_node_and_descendants(&child_id, limits);
}
}
}

View file

@ -178,12 +178,15 @@ impl NodeCountsV3 {
let mut remaining_reviews = remaining.review.saturating_sub(capped.interday_learning);
// any remaining review limit is applied to reviews
capped.review = capped.review.min(remaining_reviews);
remaining_reviews = remaining_reviews.saturating_sub(capped.review);
// new cards last, capped to new and remaining review limits
capped.new = capped.new.min(remaining_reviews).min(remaining.new);
capped.new = capped.new.min(remaining.new);
if remaining.cap_new_to_review {
remaining_reviews = remaining_reviews.saturating_sub(capped.review);
capped.new = capped.new.min(remaining_reviews);
}
capped
}
}
impl AddAssign for NodeCountsV3 {
fn add_assign(&mut self, rhs: Self) {
self.new += rhs.new;
@ -323,10 +326,18 @@ impl Collection {
let learn_cutoff = (timestamp.0 as u32) + self.learn_ahead_secs();
let sched_ver = self.scheduler_version();
let v3 = self.get_config_bool(BoolKey::Sched2021);
let new_cards_ignore_review_limit =
self.get_config_bool(BoolKey::NewCardsIgnoreReviewLimit);
let counts = self.due_counts(days_elapsed, learn_cutoff)?;
let dconf = self.storage.get_deck_config_map()?;
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,
new_cards_ignore_review_limit,
);
if sched_ver == SchedulerVersion::V2 {
if v3 {
sum_counts_and_apply_limits_v3(&mut tree, &limits);

View file

@ -278,7 +278,7 @@ mod test {
let mut data = ExchangeData::default();
let mut col = open_test_collection();
let note = col.add_new_note("Basic");
let note = NoteAdder::basic(&mut col).add(&mut col);
data.gather_data(&mut col, SearchNode::WholeCollection, true)
.unwrap();
@ -291,7 +291,7 @@ mod test {
let mut col = open_test_collection();
let now_micros = TimestampMillis::now().0 * 1000;
let mut note = col.add_new_note("Basic");
let mut note = NoteAdder::basic(&mut col).add(&mut col);
note.id = NoteId(now_micros);
col.add_note_only_with_id_undoable(&mut note).unwrap();

View file

@ -210,24 +210,23 @@ mod test {
use super::*;
use crate::collection::open_test_collection;
use crate::tests::new_deck_with_machine_name;
#[test]
fn parents() {
let mut col = open_test_collection();
col.add_deck_with_machine_name("filtered", true);
col.add_deck_with_machine_name("PARENT", false);
DeckAdder::new("filtered").filtered(true).add(&mut col);
DeckAdder::new("PARENT").add(&mut col);
let mut ctx = DeckContext::new(&mut col, Usn(1));
ctx.unique_suffix = "".to_string();
let imports = vec![
new_deck_with_machine_name("unknown parent\x1fchild", false),
new_deck_with_machine_name("filtered\x1fchild", false),
new_deck_with_machine_name("parent\x1fchild", false),
new_deck_with_machine_name("NEW PARENT\x1fchild", false),
new_deck_with_machine_name("new parent", false),
DeckAdder::new("unknown parent\x1fchild").deck(),
DeckAdder::new("filtered\x1fchild").deck(),
DeckAdder::new("parent\x1fchild").deck(),
DeckAdder::new("NEW PARENT\x1fchild").deck(),
DeckAdder::new("new parent").deck(),
];
ctx.import_decks(imports, false, false).unwrap();
let existing_decks: HashSet<_> = ctx

View file

@ -342,7 +342,7 @@ mod test {
#[test]
fn should_add_note_with_new_id_if_guid_is_unique_and_id_is_not() {
let mut col = open_test_collection();
let mut note = col.add_new_note("basic");
let mut note = NoteAdder::basic(&mut col).add(&mut col);
note.guid = "other".to_string();
let original_id = note.id;
@ -354,7 +354,7 @@ mod test {
#[test]
fn should_skip_note_if_guid_already_exists_with_newer_mtime() {
let mut col = open_test_collection();
let mut note = col.add_new_note("basic");
let mut note = NoteAdder::basic(&mut col).add(&mut col);
note.mtime.0 -= 1;
note.fields_mut()[0] = "outdated".to_string();
@ -366,7 +366,7 @@ mod test {
#[test]
fn should_update_note_if_guid_already_exists_with_different_id() {
let mut col = open_test_collection();
let mut note = col.add_new_note("basic");
let mut note = NoteAdder::basic(&mut col).add(&mut col);
note.id.0 = 42;
note.mtime.0 += 1;
note.fields_mut()[0] = "updated".to_string();
@ -379,7 +379,7 @@ mod test {
#[test]
fn should_ignore_note_if_guid_already_exists_with_different_notetype() {
let mut col = open_test_collection();
let mut note = col.add_new_note("basic");
let mut note = NoteAdder::basic(&mut col).add(&mut col);
note.notetype_id.0 = 42;
note.mtime.0 += 1;
note.fields_mut()[0] = "updated".to_string();
@ -393,7 +393,7 @@ mod test {
fn should_add_note_with_remapped_notetype_if_in_notetype_map() {
let mut col = open_test_collection();
let basic_ntid = col.get_notetype_by_name("basic").unwrap().unwrap().id;
let mut note = col.new_note("basic");
let mut note = NoteAdder::basic(&mut col).note();
note.notetype_id.0 = 123;
let mut log = import_note!(col, note, NotetypeId(123) => basic_ntid);
@ -405,7 +405,7 @@ mod test {
fn should_ignore_note_if_guid_already_exists_and_notetype_is_remapped() {
let mut col = open_test_collection();
let basic_ntid = col.get_notetype_by_name("basic").unwrap().unwrap().id;
let mut note = col.add_new_note("basic");
let mut note = NoteAdder::basic(&mut col).add(&mut col);
note.notetype_id.0 = 123;
note.mtime.0 += 1;
note.fields_mut()[0] = "updated".to_string();
@ -418,7 +418,7 @@ mod test {
#[test]
fn should_add_note_with_remapped_media_reference_in_field_if_in_media_map() {
let mut col = open_test_collection();
let mut note = col.new_note("basic");
let mut note = NoteAdder::basic(&mut col).note();
note.fields_mut()[0] = "<img src='foo.jpg'>".to_string();
let mut media_map = MediaUseMap::default();

View file

@ -717,7 +717,9 @@ mod test {
#[test]
fn should_recognize_normalized_duplicate_only_if_normalization_is_enabled() {
let mut col = open_test_collection();
col.add_new_note_with_fields("Basic", &["", "old"]);
NoteAdder::basic(&mut col)
.fields(&["", "old"])
.add(&mut col);
let mut data = ForeignData::with_defaults();
data.dupe_resolution = DupeResolution::Update;
data.add_note(&["", "new"]);

View file

@ -161,7 +161,11 @@ impl OpChanges {
let c = &self.changes;
(c.card && self.op != Op::SetFlag)
|| c.deck
|| (c.config && matches!(self.op, Op::SetCurrentDeck | Op::UpdatePreferences))
|| (c.config
&& matches!(
self.op,
Op::SetCurrentDeck | Op::UpdatePreferences | Op::UpdateDeckConfig
))
|| c.deck_config
}
}

View file

@ -31,6 +31,8 @@ pub use crate::require;
pub use crate::revlog::RevlogId;
pub use crate::search::SearchBuilder;
pub use crate::search::TryIntoSearch;
#[cfg(test)]
pub(crate) use crate::tests::*;
pub use crate::timestamp::TimestampMillis;
pub use crate::timestamp::TimestampSecs;
pub(crate) use crate::types::IntoNewtypeVec;

View file

@ -5,6 +5,7 @@ use super::DueCard;
use super::NewCard;
use super::QueueBuilder;
use crate::deckconfig::NewCardGatherPriority;
use crate::decks::limits::LimitKind;
use crate::prelude::*;
use crate::scheduler::queue::DueCardKind;
use crate::storage::card::NewCardSorting;
@ -14,7 +15,6 @@ impl QueueBuilder {
self.gather_intraday_learning_cards(col)?;
self.gather_due_cards(col, DueCardKind::Learning)?;
self.gather_due_cards(col, DueCardKind::Review)?;
self.limits.cap_new_to_review();
self.gather_new_cards(col)?;
Ok(())
@ -33,27 +33,30 @@ impl QueueBuilder {
}
fn gather_due_cards(&mut self, col: &mut Collection, kind: DueCardKind) -> Result<()> {
if !self.limits.root_limit_reached() {
col.storage.for_each_due_card_in_active_decks(
self.context.timing.days_elapsed,
self.context.sort_options.review_order,
kind,
|card| {
if self.limits.root_limit_reached() {
false
} else {
if let Some(node_id) = self.limits.remaining_node_id(card.current_deck_id) {
if self.add_due_card(card) {
self.limits
.decrement_node_and_parent_limits(&node_id, false);
}
}
true
}
},
)?;
if self.limits.root_limit_reached(LimitKind::Review) {
return Ok(());
}
Ok(())
col.storage.for_each_due_card_in_active_decks(
self.context.timing.days_elapsed,
self.context.sort_options.review_order,
kind,
|card| {
if self.limits.root_limit_reached(LimitKind::Review) {
return Ok(false);
}
if !self
.limits
.limit_reached(card.current_deck_id, LimitKind::Review)?
&& self.add_due_card(card)
{
self.limits.decrement_deck_and_parent_limits(
card.current_deck_id,
LimitKind::Review,
)?;
}
Ok(true)
},
)
}
fn gather_new_cards(&mut self, col: &mut Collection) -> Result<()> {
@ -78,21 +81,20 @@ impl QueueBuilder {
fn gather_new_cards_by_deck(&mut self, col: &mut Collection) -> Result<()> {
for deck_id in self.limits.active_decks() {
if self.limits.root_limit_reached() {
if self.limits.root_limit_reached(LimitKind::New) {
break;
}
if !self.limits.limit_reached(deck_id) {
col.storage.for_each_new_card_in_deck(deck_id, |card| {
if let Some(node_id) = self.limits.remaining_node_id(deck_id) {
if self.add_new_card(card) {
self.limits.decrement_node_and_parent_limits(&node_id, true);
}
true
} else {
false
}
})?;
if self.limits.limit_reached(deck_id, LimitKind::New)? {
continue;
}
col.storage.for_each_new_card_in_deck(deck_id, |card| {
let limit_reached = self.limits.limit_reached(deck_id, LimitKind::New)?;
if !limit_reached && self.add_new_card(card) {
self.limits
.decrement_deck_and_parent_limits(deck_id, LimitKind::New)?;
}
Ok(!limit_reached)
})?;
}
Ok(())
@ -105,19 +107,19 @@ impl QueueBuilder {
) -> Result<()> {
col.storage
.for_each_new_card_in_active_decks(order, |card| {
if self.limits.root_limit_reached() {
false
} else {
if let Some(node_id) = self.limits.remaining_node_id(card.current_deck_id) {
if self.add_new_card(card) {
self.limits.decrement_node_and_parent_limits(&node_id, true);
}
}
true
if self.limits.root_limit_reached(LimitKind::New) {
return Ok(false);
}
})?;
Ok(())
if !self
.limits
.limit_reached(card.current_deck_id, LimitKind::New)?
&& self.add_new_card(card)
{
self.limits
.decrement_deck_and_parent_limits(card.current_deck_id, LimitKind::New)?;
}
Ok(true)
})
}
/// True if limit should be decremented.

View file

@ -123,10 +123,17 @@ struct Context {
impl QueueBuilder {
pub(super) fn new(col: &mut Collection, deck_id: DeckId) -> Result<Self> {
let timing = col.timing_for_timestamp(TimestampSecs::now())?;
let new_cards_ignore_review_limit = col.get_config_bool(BoolKey::NewCardsIgnoreReviewLimit);
let config_map = col.storage.get_deck_config_map()?;
let root_deck = col.storage.get_deck(deck_id)?.or_not_found(deck_id)?;
let child_decks = col.storage.child_decks(&root_deck)?;
let limits = LimitTreeMap::build(&root_deck, child_decks, &config_map, timing.days_elapsed);
let limits = LimitTreeMap::build(
&root_deck,
child_decks,
&config_map,
timing.days_elapsed,
new_cards_ignore_review_limit,
);
let sort_options = sort_options(&root_deck, &config_map);
let deck_map = col.storage.get_decks_map()?;
@ -325,38 +332,27 @@ mod test {
#[test]
fn should_build_empty_queue_if_limit_is_reached() {
let mut col = open_test_collection();
col.set_config_bool(BoolKey::Sched2021, true, false)
.unwrap();
let note_id = col.add_new_note("Basic").id;
let cids = col.storage.card_ids_of_notes(&[note_id]).unwrap();
col.set_due_date(&cids, "0", None).unwrap();
let mut col = Collection::new_v3();
CardAdder::new().due_dates(["0"]).add(&mut col);
col.set_deck_review_limit(DeckId(1), 0);
assert_eq!(col.queue_as_deck_and_template(DeckId(1)), vec![]);
}
#[test]
fn new_queue_building() -> Result<()> {
let mut col = open_test_collection();
col.set_config_bool(BoolKey::Sched2021, true, false)?;
let mut col = Collection::new_v3();
// parent
// ┣━━child━━grandchild
// ┗━━child_2
let mut parent = col.get_or_create_normal_deck("Default").unwrap();
let mut child = col.get_or_create_normal_deck("Default::child").unwrap();
let child_2 = col.get_or_create_normal_deck("Default::child_2").unwrap();
let grandchild = col
.get_or_create_normal_deck("Default::child::grandchild")
.unwrap();
let mut parent = DeckAdder::new("parent").add(&mut col);
let mut child = DeckAdder::new("parent\x1fchild").add(&mut col);
let child_2 = DeckAdder::new("parent\x1fchild_2").add(&mut col);
let grandchild = DeckAdder::new("parent\x1fchild\x1fgrandchild").add(&mut col);
// add 2 new cards to each deck
let nt = col.get_notetype_by_name("Cloze")?.unwrap();
let mut note = nt.new_note();
note.set_field(0, "{{c1::}} {{c2::}}")?;
for deck in [&parent, &child, &child_2, &grandchild] {
note.id.0 = 0;
col.add_note(&mut note, deck.id)?;
CardAdder::new().siblings(2).deck(deck.id).add(&mut col);
}
// set child's new limit to 3, which should affect grandchild
@ -462,9 +458,7 @@ mod test {
fn new_card_potentially_burying_review_card() {
let mut col = open_test_collection();
// add one new and one review card
col.add_new_note_with_fields("basic (and reversed card)", &["foo", "bar"]);
let card = col.get_first_card();
col.set_due_date(&[card.id], "0", None).unwrap();
CardAdder::new().siblings(2).due_dates(["0"]).add(&mut col);
// Potentially problematic config: New cards are shown first and would bury
// review siblings. This poses a problem because we gather review cards first.
col.update_default_deck_config(|config| {
@ -482,4 +476,18 @@ mod test {
// include the buried card.
assert_eq!(col.card_queue_len(), old_queue_len - 1);
}
#[test]
fn new_cards_may_ignore_review_limit() {
let mut col = Collection::new_v3();
col.set_config_bool(BoolKey::NewCardsIgnoreReviewLimit, true, false)
.unwrap();
col.update_default_deck_config(|config| {
config.reviews_per_day = 0;
});
CardAdder::new().add(&mut col);
// review limit doesn't apply to new card
assert_eq!(col.card_queue_len(), 1);
}
}

View file

@ -254,7 +254,7 @@ impl super::SqliteStorage {
mut func: F,
) -> Result<()>
where
F: FnMut(DueCard) -> bool,
F: FnMut(DueCard) -> Result<bool>,
{
let order_clause = review_order_sql(order, day_cutoff);
let mut stmt = self.db.prepare_cached(&format!(
@ -276,7 +276,7 @@ impl super::SqliteStorage {
current_deck_id: row.get(5)?,
original_deck_id: row.get(6)?,
kind,
}) {
})? {
break;
}
}
@ -288,7 +288,7 @@ impl super::SqliteStorage {
/// returns or no more cards found.
pub(crate) fn for_each_new_card_in_deck<F>(&self, deck: DeckId, mut func: F) -> Result<()>
where
F: FnMut(NewCard) -> bool,
F: FnMut(NewCard) -> Result<bool>,
{
let mut stmt = self.db.prepare_cached(&format!(
"{} ORDER BY due, ord ASC",
@ -296,7 +296,7 @@ impl super::SqliteStorage {
))?;
let mut rows = stmt.query(params![deck])?;
while let Some(row) = rows.next()? {
if !func(row_to_new_card(row)?) {
if !func(row_to_new_card(row)?)? {
break;
}
}
@ -312,7 +312,7 @@ impl super::SqliteStorage {
mut func: F,
) -> Result<()>
where
F: FnMut(NewCard) -> bool,
F: FnMut(NewCard) -> Result<bool>,
{
let mut stmt = self.db.prepare_cached(&format!(
"{} ORDER BY {}",
@ -321,7 +321,7 @@ impl super::SqliteStorage {
))?;
let mut rows = stmt.query(params![])?;
while let Some(row) = rows.next()? {
if !func(row_to_new_card(row)?) {
if !func(row_to_new_card(row)?)? {
break;
}
}

View file

@ -2,17 +2,17 @@
// License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
#![cfg(test)]
#![allow(dead_code)]
use itertools::Itertools;
use tempfile::tempdir;
use tempfile::TempDir;
use crate::collection::open_test_collection;
use crate::collection::CollectionBuilder;
use crate::deckconfig::DeckConfigInner;
use crate::deckconfig::UpdateDeckConfigsRequest;
use crate::io::create_dir;
use crate::media::MediaManager;
use crate::pb::deckconfig::deck_configs_for_update::current_deck::Limits;
use crate::prelude::*;
pub(crate) fn open_fs_test_collection(name: &str) -> (Collection, TempDir) {
@ -29,14 +29,14 @@ pub(crate) fn open_fs_test_collection(name: &str) -> (Collection, TempDir) {
pub(crate) fn open_test_collection_with_learning_card() -> Collection {
let mut col = open_test_collection();
col.add_new_note("basic");
NoteAdder::basic(&mut col).add(&mut col);
col.answer_again();
col
}
pub(crate) fn open_test_collection_with_relearning_card() -> Collection {
let mut col = open_test_collection();
col.add_new_note("basic");
NoteAdder::basic(&mut col).add(&mut col);
col.answer_easy();
col.storage
.db
@ -48,6 +48,13 @@ pub(crate) fn open_test_collection_with_relearning_card() -> Collection {
}
impl Collection {
pub(crate) fn new_v3() -> Collection {
let mut col = open_test_collection();
col.set_config_bool(BoolKey::Sched2021, true, false)
.unwrap();
col
}
pub(crate) fn add_media(&self, media: &[(&str, &[u8])]) {
let mgr = MediaManager::new(&self.media_folder, &self.media_db).unwrap();
for (name, data) in media {
@ -55,36 +62,10 @@ impl Collection {
}
}
pub(crate) fn new_note(&mut self, notetype: &str) -> Note {
self.get_notetype_by_name(notetype)
.unwrap()
.unwrap()
.new_note()
}
pub(crate) fn add_new_note(&mut self, notetype: &str) -> Note {
let mut note = self.new_note(notetype);
self.add_note(&mut note, DeckId(1)).unwrap();
note
}
pub(crate) fn add_new_note_with_fields(&mut self, notetype: &str, fields: &[&str]) -> Note {
let mut note = self.new_note(notetype);
*note.fields_mut() = fields.iter().map(ToString::to_string).collect();
self.add_note(&mut note, DeckId(1)).unwrap();
note
}
pub(crate) fn get_all_notes(&mut self) -> Vec<Note> {
self.storage.get_all_notes()
}
pub(crate) fn add_deck_with_machine_name(&mut self, name: &str, filtered: bool) -> Deck {
let mut deck = new_deck_with_machine_name(name, filtered);
self.add_deck_inner(&mut deck, Usn(1)).unwrap();
deck
}
pub(crate) fn get_first_card(&self) -> Card {
self.storage.get_all_cards().pop().unwrap()
}
@ -97,35 +78,27 @@ impl Collection {
self.update_default_deck_config(|config| config.relearn_steps = steps);
}
/// Updates with the modified config, then resorts and adjusts remaining
/// steps in the default deck.
pub(crate) fn update_default_deck_config(
&mut self,
modifier: impl FnOnce(&mut DeckConfigInner),
) {
let mut config = self
let config = self
.get_deck_config(DeckConfigId(1), false)
.unwrap()
.unwrap();
modifier(&mut config.inner);
self.update_deck_configs(UpdateDeckConfigsRequest {
target_deck_id: DeckId(1),
configs: vec![config],
removed_config_ids: vec![],
apply_to_children: false,
card_state_customizer: "".to_string(),
limits: Limits::default(),
})
.unwrap();
}
}
let mut new_config = config.clone();
pub(crate) fn new_deck_with_machine_name(name: &str, filtered: bool) -> Deck {
let mut deck = if filtered {
Deck::new_filtered()
} else {
Deck::new_normal()
};
deck.name = NativeDeckName::from_native_str(name);
deck
modifier(&mut new_config.inner);
self.update_deck_config_inner(&mut new_config, config.clone(), None)
.unwrap();
self.sort_deck(DeckId(1), config.inner.new_card_insert_order(), Usn(0))
.unwrap();
self.adjust_remaining_steps_in_deck(DeckId(1), Some(&config), Some(&new_config), Usn(0))
.unwrap();
}
}
#[derive(Debug, Default, Clone)]
@ -143,7 +116,6 @@ impl DeckAdder {
}
}
#[allow(dead_code)]
pub(crate) fn filtered(mut self, filtered: bool) -> Self {
self.filtered = filtered;
self
@ -156,14 +128,10 @@ impl DeckAdder {
self
}
pub(crate) fn add(self, col: &mut Collection) -> Deck {
let mut deck = if self.filtered {
Deck::new_filtered()
} else {
Deck::new_normal()
};
deck.name = self.name;
if let Some(mut config) = self.config {
pub(crate) fn add(mut self, col: &mut Collection) -> Deck {
let config_opt = self.config.take();
let mut deck = self.deck();
if let Some(mut config) = config_opt {
col.add_or_update_deck_config(&mut config).unwrap();
deck.normal_mut()
.expect("can't set config for filtered deck")
@ -172,6 +140,16 @@ impl DeckAdder {
col.add_or_update_deck(&mut deck).unwrap();
deck
}
pub(crate) fn deck(self) -> Deck {
let mut deck = if self.filtered {
Deck::new_filtered()
} else {
Deck::new_normal()
};
deck.name = self.name;
deck
}
}
#[derive(Debug, Clone)]
@ -183,7 +161,11 @@ pub(crate) struct NoteAdder {
impl NoteAdder {
pub(crate) fn new(col: &mut Collection, notetype: &str) -> Self {
Self {
note: col.new_note(notetype),
note: col
.get_notetype_by_name(notetype)
.unwrap()
.unwrap()
.new_note(),
deck: DeckId(1),
}
}
@ -192,6 +174,10 @@ impl NoteAdder {
Self::new(col, "basic")
}
pub(crate) fn cloze(col: &mut Collection) -> Self {
Self::new(col, "cloze")
}
pub(crate) fn fields(mut self, fields: &[&str]) -> Self {
*self.note.fields_mut() = fields.iter().map(ToString::to_string).collect();
self
@ -206,4 +192,64 @@ impl NoteAdder {
col.add_note(&mut self.note, self.deck).unwrap();
self.note
}
pub(crate) fn note(self) -> Note {
self.note
}
}
#[derive(Debug, Clone)]
pub(crate) struct CardAdder {
siblings: usize,
deck: DeckId,
due_dates: Vec<&'static str>,
}
impl CardAdder {
pub(crate) fn new() -> Self {
Self {
siblings: 1,
deck: DeckId(1),
due_dates: Vec::new(),
}
}
pub(crate) fn siblings(mut self, siblings: usize) -> Self {
self.siblings = siblings;
self
}
pub(crate) fn deck(mut self, deck: DeckId) -> Self {
self.deck = deck;
self
}
/// Takes an array of strs and sets the due date of the first siblings
/// accordingly, skipping siblings if a str is empty.
pub(crate) fn due_dates(mut self, due_dates: impl Into<Vec<&'static str>>) -> Self {
self.due_dates = due_dates.into();
self
}
pub(crate) fn add(&self, col: &mut Collection) -> Vec<Card> {
let field = (1..self.siblings + 1)
.map(|n| format!("{{{{c{n}::}}}}"))
.join("");
let note = NoteAdder::cloze(col)
.fields(&[&field, ""])
.deck(self.deck)
.add(col);
if !self.due_dates.is_empty() {
let cids = col.storage.card_ids_of_notes(&[note.id]).unwrap();
for (ord, due_date) in self.due_dates.iter().enumerate() {
if !due_date.is_empty() {
col.set_due_date(&cids[ord..ord + 1], due_date, None)
.unwrap();
}
}
}
col.storage.all_cards_of_note(note.id).unwrap()
}
}

View file

@ -15,6 +15,7 @@
import { ValueTab } from "./lib";
import SettingTitle from "./SettingTitle.svelte";
import SpinBoxRow from "./SpinBoxRow.svelte";
import SwitchRow from "./SwitchRow.svelte";
import TabbedValue from "./TabbedValue.svelte";
import type { DeckOption } from "./types";
import Warning from "./Warning.svelte";
@ -43,17 +44,18 @@
const limits = state.deckLimits;
const defaults = state.defaults;
const parentLimits = state.parentLimits;
const newCardsIgnoreReviewLimit = state.newCardsIgnoreReviewLimit;
const v3Extra = state.v3Scheduler
? "\n\n" +
tr.deckConfigLimitNewBoundByReviews() +
"\n\n" +
tr.deckConfigLimitInterdayBoundByReviews() +
"\n\n" +
tr.deckConfigLimitDeckV3() +
"\n\n" +
tr.deckConfigTabDescription()
? "\n\n" + tr.deckConfigLimitDeckV3() + "\n\n" + tr.deckConfigTabDescription()
: "";
const reviewV3Extra = state.v3Scheduler
? "\n\n" + tr.deckConfigLimitInterdayBoundByReviews() + v3Extra
: "";
const newCardsIgnoreReviewLimitHelp =
tr.deckConfigAffectsEntireCollection() +
"\n\n" +
tr.deckConfigNewCardsIgnoreReviewLimitTooltip();
$: newCardsGreaterThanParent =
!state.v3Scheduler && newValue > $parentLimits.newCards
@ -137,9 +139,14 @@
},
reviewLimit: {
title: tr.schedulingMaximumReviewsday(),
help: tr.deckConfigReviewLimitTooltip() + v3Extra,
help: tr.deckConfigReviewLimitTooltip() + reviewV3Extra,
url: "https://docs.ankiweb.net/deck-options.html#maximum-reviewsday",
},
newCardsIgnoreReviewLimit: {
title: tr.deckConfigNewCardsIgnoreReviewLimit(),
help: newCardsIgnoreReviewLimitHelp,
url: "https://docs.ankiweb.net/deck-options.html#new-cardsday",
},
};
const helpSections = Object.values(settings) as DeckOption[];
@ -192,5 +199,18 @@
<Item>
<Warning warning={reviewsTooLow} />
</Item>
{#if state.v3Scheduler}
<Item>
<SwitchRow bind:value={$newCardsIgnoreReviewLimit} defaultValue={false}>
<SettingTitle
on:click={() =>
openHelpModal(
Object.keys(settings).indexOf("newIgnoreReviewLimit"),
)}>{settings.newCardsIgnoreReviewLimit.title}</SettingTitle
>
</SwitchRow>
</Item>
{/if}
</DynamicallySlottable>
</TitledContainer>

View file

@ -40,6 +40,7 @@ export class DeckOptionsState {
readonly defaults: DeckConfig.DeckConfig.Config;
readonly addonComponents: Writable<DynamicSvelteComponent[]>;
readonly v3Scheduler: boolean;
readonly newCardsIgnoreReviewLimit: Writable<boolean>;
private targetDeckId: number;
private configs: ConfigWithCount[];
@ -70,6 +71,7 @@ export class DeckOptionsState {
this.v3Scheduler = data.v3Scheduler;
this.cardStateCustomizer = writable(data.cardStateCustomizer);
this.deckLimits = writable(data.currentDeck?.limits ?? createLimits());
this.newCardsIgnoreReviewLimit = writable(data.newCardsIgnoreReviewLimit);
// decrement the use count of the starting item, as we'll apply +1 to currently
// selected one at display time
@ -195,6 +197,7 @@ export class DeckOptionsState {
applyToChildren,
cardStateCustomizer: get(this.cardStateCustomizer),
limits: get(this.deckLimits),
newCardsIgnoreReviewLimit: get(this.newCardsIgnoreReviewLimit),
};
}