Don't allow duplicate category names (per group) #1833

Shazib · 2023-10-28T19:49:23Z

GUI Level check and a separate one at DB to catch API etc.

netlify · 2023-10-28T19:49:27Z

✅ Deploy Preview for actualbudget ready!

Name	Link
🔨 Latest commit	`a40e607`
🔍 Latest deploy log	https://app.netlify.com/sites/actualbudget/deploys/655a34143e173c00080ab308
😎 Deploy Preview	https://deploy-preview-1833.demo.actualbudget.org/
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

github-actions · 2023-10-28T19:53:08Z

Bundle Stats — desktop-client

Hey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle.

As this PR is updated, I'll keep you updated on how the bundle size is impacted.

Total

Files count	Total bundle size	% Changed
16	2.97 MB → 2.97 MB (+566 B)	+0.02%

Changeset

File	Δ	Size
`src/components/budget/index.tsx`	📈 +1.03 kB (+6.41%)	16.08 kB → 17.11 kB

View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

Asset	File Size	% Changed
static/js/wide-components.chunk.js	117.06 kB → 117.61 kB (+566 B)	+0.47%

Smaller

No assets were smaller

Unchanged

Asset	File Size	% Changed
static/js/main.js	1.16 MB	0%
static/js/665.chunk.js	824.28 kB	0%
static/media/Inter-italic.var.woff2	239.29 kB	0%
static/media/Inter-roman.var.woff2	221.86 kB	0%
static/js/444.chunk.js	156.11 kB	0%
static/js/231.chunk.js	117.37 kB	0%
static/js/reports.chunk.js	74.37 kB	0%
static/js/narrow-components.chunk.js	52.67 kB	0%
static/js/301.chunk.js	13.31 kB	0%
static/js/473.chunk.js	13 kB	0%
static/js/resize-observer-polyfill.chunk.js	8.88 kB	0%
static/css/main.css	7.41 kB	0%
asset-manifest.json	2.07 kB	0%
index.html	1.66 kB	0%
static/media/browser-server.js	903 B	0%

github-actions · 2023-10-28T19:53:10Z

Bundle Stats — loot-core

Hey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle.

As this PR is updated, I'll keep you updated on how the bundle size is impacted.

Total

Files count	Total bundle size	% Changed
2	2.22 MB → 2.22 MB (+226 B)	+0.01%

Changeset

File	Δ	Size
`packages/loot-core/src/server/db/index.ts`	📈 +419 B (+2.41%)	17 kB → 17.41 kB

View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

Asset	File Size	% Changed
kcab.worker.js	1.23 MB → 1.23 MB (+226 B)	+0.02%

Smaller

No assets were smaller

Unchanged

Asset	File Size	% Changed
xfo.xfo.kcab.worker.js	1014.46 kB	0%

rich-howell · 2023-10-28T20:14:44Z

Hey @Shazib

This doesn't actually fix the problem though, I could create Food in one group then hide it and create it later in another group and it would still show me two Food's on the transaction entry.

I would have to go back to the budget check which group the category is showing for then go back to the transaction entry screen and enter the transaction..seems like a lot of clicking around.

What would the use case be for two categories on the budget being called the same thing ever existing? For example, would this be a possibility?

Usual Expenses - Food
Bills - Food

If it wouldn't it might be worth preventing duplicate category names entirely?

Shazib · 2023-10-28T20:58:01Z

If it wouldn't it might be worth preventing duplicate category names entirely?

Hi @rich-howell,

I can envisage scenarios where you have multiple of the same categories under different groups

e.g.
"Work Expenses" -> "Food"
"Travel" -> "Food"
"Household" -> "Food"

Or
"Holidays" -> "Travel Costs"
"Work" -> "Travel Costs"

~~Maybe the popup could show you the group as well?~~

Shazib · 2023-10-28T21:05:14Z

~~Maybe the popup could show you the group as well?~~

Maybe the popup could indicate hidden categories?

Shazib · 2023-10-28T21:15:13Z

@rich-howell what do you think

Crazypkr1099 · 2023-10-28T21:16:56Z

@rich-howell what do you think

I don't like this. It's going to clutter the crap out of it. Honestly I don't see the big deal with checking your hiddens before creating a group 🤷

Shazib · 2023-10-28T21:18:12Z

I don't like this. It's going to clutter the crap out of it. Honestly I don't see the big deal with checking your hiddens before creating a group 🤷

Do people have loads of hidden categories? I wouldnt consider that super cluttered personally!

Shazib · 2023-10-28T21:21:26Z

I actually think its kind of useful to see if its hidden 🤷

Although maybe this is kind of two separate PR's; not allowing duplicate names is fairly straightforward.

youngcw · 2023-10-28T21:52:53Z

I think the category picker change should be a separate PR.

Have you checked the case of moving a category from one group to another? I would be ok with allowing same names but different groups, but then we need to do the checks on moves as well as creates. That wouldn't matter if only one of the same name are allowed.

Shazib · 2023-10-28T22:04:39Z

I think the category picker change should be a separate PR.

Have you checked the case of moving a category from one group to another? I would be ok with allowing same names but different groups, but then we need to do the checks on moves as well as creates. That wouldn't matter if only one of the same name are allowed.

The change isn't actually in the PR just a screenshot.

I will check the case of moving a category!

rich-howell · 2023-10-29T06:57:13Z

@rich-howell what do you think

I don't like this. It's going to clutter the crap out of it. Honestly I don't see the big deal with checking your hiddens before creating a group 🤷

There are two workflows here that are impacted by hidden categories and the ability to create a category with a duplicate name.

The Budget Sheet

If you have a category you created 2 years ago and it is hidden your not going to remember that you hid it before creating a new one, I don't think the workflow of checking if the category is hidden before creating any new category really works all that well.

Transaction Sheet

When adding a new transaction, the category shows regardless of if it is hidden or not so if you select the category you hid two years ago and remember you might not know it is there you will be thinking why is my budget now showing negative numbers.

My thoughts

I agree that seeing if the category is hidden in the category picker is an improvement on the current workflow and would solve the problem entirely. (You might not even have to stop people creating duplicates)

youngcw · 2023-11-07T20:13:30Z

This seems to be working well. Can you look at getting the tests to pass, and the merge conflicts cleared up?

carkom · 2023-11-08T11:07:47Z

This is great @Shazib. Maybe you'd be interested in doing the same for account names? If not, no worries. Cheers!

Shazib · 2023-11-10T17:01:09Z

For point 1, Can't you just hit escape?

That doesn't close the category creation view, just clears the filled in name.

Yes sorry - you can just click off it though, you don't have to go through with the create, it cancels if you focus away from the cell

Shazib · 2023-11-10T17:02:22Z

This still doesn't fix the problem in that I raised in the issue, it is still not clear in the category drop down which category is hidden and which isn't (screenshot taken from the deploy preview)

I have removed the tag linking that issue.

I think this is safe to merge standalone while we work on that.

…o category-guard

youngcw · 2023-11-16T17:05:18Z

@Shazib This is pretty much ready right? If you merge master the electron issues should be fixed.

Shazib · 2023-11-16T17:45:55Z

@Shazib This is pretty much ready right? If you merge master the electron issues should be fixed.

@youngcw yep. I'll merge master tonight.

youngcw · 2023-11-16T23:48:10Z

Should this check for same name, different case? ie not capitalized

Shazib · 2023-11-17T00:06:27Z

Should this check for same name, different case? ie not capitalized

good spot - added

carkom · 2023-11-17T10:47:30Z

This is great @Shazib. Maybe you'd be interested in doing the same for account names? If not, no worries. Cheers!

Might be good to also do group names? Could be a separate PR.

Shazib · 2023-11-17T14:46:04Z

This is great @Shazib. Maybe you'd be interested in doing the same for account names? If not, no worries. Cheers!

Might be good to also do group names? Could be a separate PR.

Yeh i'd do that as a separate PR to avoid scope creep with this one.

* Remove author from electron package.json * Don't allow dupes in cat names (per group) * Release Notes * Handle reorders and moves * Fix linter warnings * Fix typecheck * Show the name of the duplicate category * missed func call * Upper case before compare * lint fixes --------- Co-authored-by: Shazib Hussain <[email protected]>

Shazib added 3 commits July 29, 2023 22:02

Remove author from electron package.json

a141756

Merge branch 'actualbudget:master' into master

67159a6

Don't allow dupes in cat names (per group)

d41aa86

trafico-bot bot added the 🔍 Ready for Review label Oct 28, 2023

Release Notes

7aaf7d4

Shazib changed the title ~~Don't allow duplicate category names (per group)~~ [WIP] Don't allow duplicate category names (per group) Oct 28, 2023

trafico-bot bot added 🚧 WIP and removed 🔍 Ready for Review labels Oct 28, 2023

Handle reorders and moves

ad293c8

Shazib changed the title ~~[WIP] Don't allow duplicate category names (per group)~~ Don't allow duplicate category names (per group) Oct 29, 2023

trafico-bot bot added 🔍 Ready for Review and removed 🚧 WIP labels Oct 29, 2023

Shazib added 2 commits October 30, 2023 21:48

Merge branch 'master' into category-guard

7700b45

Merge branch 'actualbudget:master' into master

a7a8b02

Merge remote-tracking branch 'parent/master'

8b66d1c

Merge branch 'master' into category-guard

15bccc3

Shazib Hussain and others added 6 commits November 10, 2023 17:14

Show the name of the duplicate category

3728633

Merge branch 'category-guard' of https://github.com/Shazib/actual int…

e4d55f2

…o category-guard

missed func call

1915f07

Merge branch 'master' into category-guard

0b4558d

Merge branch 'master' into category-guard

55f3033

Merge branch 'master' into category-guard

96753d0

Merge branch 'master' into category-guard

3147615

Upper case before compare

8fe1aa9

lint fixes

6511065

youngcw approved these changes Nov 17, 2023

View reviewed changes

trafico-bot bot added ✅ Approved and removed 🔍 Ready for Review labels Nov 17, 2023

Shazib added 3 commits November 17, 2023 17:11

Merge branch 'master' into category-guard

c14ec2a

Merge branch 'master' into category-guard

1fa5880

Merge branch 'master' into category-guard

a40e607

youngcw merged commit 9c6d9ec into actualbudget:master Nov 20, 2023
19 checks passed

trafico-bot bot added ✨ Merged Pull Request has been merged successfully and removed ✅ Approved labels Nov 20, 2023

dhruvramdev mentioned this pull request Jan 20, 2024

feat: Don't allow duplicate cat-groups in budget #2262

Merged

Shazib deleted the category-guard branch May 24, 2024 18:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't allow duplicate category names (per group) #1833

Don't allow duplicate category names (per group) #1833

Shazib commented Oct 28, 2023 •

edited

Loading

netlify bot commented Oct 28, 2023 •

edited

Loading

github-actions bot commented Oct 28, 2023 •

edited

Loading

github-actions bot commented Oct 28, 2023 •

edited

Loading

rich-howell commented Oct 28, 2023

Shazib commented Oct 28, 2023 •

edited

Loading

Shazib commented Oct 28, 2023

Shazib commented Oct 28, 2023

Crazypkr1099 commented Oct 28, 2023

Shazib commented Oct 28, 2023

Shazib commented Oct 28, 2023

youngcw commented Oct 28, 2023

Shazib commented Oct 28, 2023

rich-howell commented Oct 29, 2023

youngcw commented Nov 7, 2023

carkom commented Nov 8, 2023

Shazib commented Nov 10, 2023

Shazib commented Nov 10, 2023

youngcw commented Nov 16, 2023

Shazib commented Nov 16, 2023

youngcw commented Nov 16, 2023

Shazib commented Nov 17, 2023

carkom commented Nov 17, 2023

Shazib commented Nov 17, 2023

Don't allow duplicate category names (per group) #1833

Don't allow duplicate category names (per group) #1833

Conversation

Shazib commented Oct 28, 2023 • edited Loading

netlify bot commented Oct 28, 2023 • edited Loading

✅ Deploy Preview for actualbudget ready!

github-actions bot commented Oct 28, 2023 • edited Loading

Bundle Stats — desktop-client

github-actions bot commented Oct 28, 2023 • edited Loading

Bundle Stats — loot-core

rich-howell commented Oct 28, 2023

Shazib commented Oct 28, 2023 • edited Loading

Shazib commented Oct 28, 2023

Shazib commented Oct 28, 2023

Crazypkr1099 commented Oct 28, 2023

Shazib commented Oct 28, 2023

Shazib commented Oct 28, 2023

youngcw commented Oct 28, 2023

Shazib commented Oct 28, 2023

rich-howell commented Oct 29, 2023

youngcw commented Nov 7, 2023

carkom commented Nov 8, 2023

Shazib commented Nov 10, 2023

Shazib commented Nov 10, 2023

youngcw commented Nov 16, 2023

Shazib commented Nov 16, 2023

youngcw commented Nov 16, 2023

Shazib commented Nov 17, 2023

carkom commented Nov 17, 2023

Shazib commented Nov 17, 2023

Shazib commented Oct 28, 2023 •

edited

Loading

netlify bot commented Oct 28, 2023 •

edited

Loading

github-actions bot commented Oct 28, 2023 •

edited

Loading

github-actions bot commented Oct 28, 2023 •

edited

Loading

Shazib commented Oct 28, 2023 •

edited

Loading