Repository: ledger/ledger (language: cpp)

Issue:
related transactions dont work with bucket
I think `reg --related` is broken when using `bucket`.

Consider this transaction:

```
2023/01/01 * Payment
    Assets:Checking     $-34.86
    Liabilities:CreditCard
```

That works fine with `--related`:

```
$ ledger --args-only -f test.ldg reg --related ^Assets:Checking
23-Jan-01 Payment                    Liabilities:CreditCard                  $34.86          $34.86
```

I think this transaction is identical, but doesn't work:

```
bucket Liabilities:CreditCard

2023/01/01 * Payment
    Assets:Checking     $-34.86
```
```
$ ledger --args-only -f test.ldg reg --related ^Assets:Checking
```

I was using `bucket`, and noticed some reports seemed wrong.


Discussion / hints:
The problem is that `related_posts::flush()` explicitly checks for posts with the `ITEM_GENERATED`  flag and ignores them.

That comes from `xact_base_t::finalize()`:

https://github.com/ledger/ledger/blob/4f40c3056a68151f52422d54572fa72c0f19151d/src/xact.cc#L213

So the question is, are bucket posts *really* generated? I mean, I understand they're synthetic, but not in the same way that the other generated posts are.

 I just applied this patch:

```diff
diff --git a/src/xact.cc b/src/xact.cc
index 961ff010..c0b5d733 100644
--- a/src/xact.cc
+++ b/src/xact.cc
@@ -210,7 +210,7 @@ bool xact_base_t::finalize()
   // been set.
 
   if (journal && journal->bucket && posts.size() == 1 && ! balance.is_null()) {
-    null_post = new post_t(journal->bucket, ITEM_GENERATED);
+    null_post = new post_t(journal->bucket);
     null_post->_state = (*posts.begin())->_state;
     add_post(null_post);
   }

```

That seems to fix the problem and the tests pass, but is it the real solution? I'm tempted to think it is correct, browsing through all the locations that flag is tested, I *think* you would still want them to apply to buckets?

```
100% tests passed, 0 tests failed out of 411
```


@jwiegley any input on the analysis from @taviso?

Here is my argument that this change should be committed 😄 

It is self-evident that this cannot possibly break anything *unless* you're using `bucket`, which we know for sure is already not working. This change does fix the known issues with bucket, so is clearly an improvement!

If I turn this into a PR with some tests, would you be willing to merge?
friendly ping!  I'm wondering if maybe `bucket` is deprecated?
I do see sense in your reasoning, @taviso. Given my focus is currently in other parts of Ledger I very much would like to rely on @jwiegley expertise opinion on this.
Hi @taviso, the definition of `ITEM_GENERATED` is fairly clear as to intent:
```
#define ITEM_GENERATED         0x01 // posting was not found in a journal
```
What I think we should do is add another flag, `ITEM_INFERRED`, set this for bucketed items, and then I'm OK with not setting `ITEM_GENERATED`, so long as we have a way to distinguish "items the user actually typed in" from those that Ledger inferred or generated.

Fix patch:
diff --git a/NEWS.md b/NEWS.md
index 1bc29b570..07cad014c 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -9,6 +9,8 @@
 
 - Include contrib files in distribution
 
+- Fix related reports when using bucket transactions (ledger/ledger#2220)
+
 ## 3.3.2 (2023-03-30)
 
 - Fix divide by zero (ledger/ledger#777, ledger/ledger#2207)
diff --git a/src/item.h b/src/item.h
index c21450e80..84f062ab0 100644
--- a/src/item.h
+++ b/src/item.h
@@ -86,6 +86,7 @@ class item_t : public flags::supports_flags<uint_least16_t>, public scope_t
 #define ITEM_GENERATED         0x01 // posting was not found in a journal
 #define ITEM_TEMP              0x02 // posting is a managed temporary
 #define ITEM_NOTE_ON_NEXT_LINE 0x04 // did we see a note on the next line?
+#define ITEM_INFERRED          0x08 // bucketed item
 
   enum state_t { UNCLEARED = 0, CLEARED, PENDING };
 
diff --git a/src/xact.cc b/src/xact.cc
index 961ff0101..55e816cbb 100644
--- a/src/xact.cc
+++ b/src/xact.cc
@@ -210,7 +210,7 @@ bool xact_base_t::finalize()
   // been set.
 
   if (journal && journal->bucket && posts.size() == 1 && ! balance.is_null()) {
-    null_post = new post_t(journal->bucket, ITEM_GENERATED);
+    null_post = new post_t(journal->bucket, ITEM_INFERRED);
     null_post->_state = (*posts.begin())->_state;
     add_post(null_post);
   }


Test patch:
diff --git a/test/regress/issue2220.test b/test/regress/issue2220.test
new file mode 100644
index 000000000..095d0f630
--- /dev/null
+++ b/test/regress/issue2220.test
@@ -0,0 +1,8 @@
+bucket Liabilities:CreditCard
+
+2023/01/01 * Payment
+    Assets:Checking     $-34.86
+
+test reg --monthly --related ^Assets:Checking
+23-Jan-01 - 23-Jan-31           Liabilities:CreditCard       $34.86       $34.86
+end test