Open PragTob opened 1 month ago
Thanks, great compilation of our screw ups haha 😄
Some answers to your questions / feedback
As it's a lot of feedback (and good one), maybe you can add a small time/complexity assessment, in regards to which are the low hanging fruits. E.g. refactoring the views I would propose to do after my open lane PR. But solving a couple of N+1 would be a good and small improvement. What do you think?
Might be nice to start the migration soon (unless there are deadlines), as otherwise it'll always become more code to change. I think it can be done step by step. Maybe after https://github.com/b310-digital/mindwendel/pull/344 ?
yes, same thought. But we should not defer it much later.
Submitting ideas via a key combination like + would be awesome
So you mean after hitting hotkey "I" entering some data, you want "+" to submit the idea? or maybe control + enter?
Btw: I just used the live generator for the lanes which also generated the core_components
which you mentioned, see https://github.com/b310-digital/mindwendel/pull/344/commits/47c616f3a851859bc2c9c0c5a4f80a358770cf42
@JannikStreek lmao... I wrote <Ctrl> + <Enter>
but without the code fences and of course they are interpreted as HTML tags then and not shown... fixing now but yes I meant ctrl + enter
@JannikStreek as for the time/complexity/effort estimations tough to do when I don't know the app as well yet I can give it a shot but it'll be off even more than normal :)
Aight here is an overview issue, high level I haven't looked at the code in more detail yet:
Overview
Phoenix HTML style/dependency upgrade
(gave this one its own section due to its importance imo)
This relates to #310 but is more than this. The HTML usage is kind of old. There is a new "style" with heex to write components in that we don't seem to be using that seems to have been [introduced in 0.16[(https://hexdocs.pm/phoenix_live_view/bindings.html#rate-limiting-events-with-debounce-and-throttle) (we're on 0.18.18):
i.e. instead of:
what's done is:
(and I think these days you also use a function component for input so
<.input ... >
).Most notably phoenix now generates some core components (see f.ex. here - not sure when it was introduced) which is really nice and useful imo. (future Tobi: seems to have been added in 1.7 along with tailwind)
Might be nice to start the migration soon (unless there are deadlines), as otherwise it'll always become more code to change. I think it can be done step by step. Maybe after #344 ?
Performance
Full page load query logs for me
software design
Lane
uses mainly things from theBrainstorming
context, similar toIdeas
- normally the main schemas belong to the context, but untangling that is always tough. PerhapsBrainstorming
is just too big of a context as it encompasses almost all? :thinking:Code Style
Strict mode credo output
``` # mix credo --strict Checking 82 source files (this might take a while) ... Software Design ┃ ┃ [D] ↘ Nested modules could be aliased at the top of the invoking module. ┃ lib/mindwendel_web/live/brainstorming_live/show.ex:13:23 #(MindwendelWeb.BrainstormingLive.Show.mount) ┃ [D] ↘ Nested modules could be aliased at the top of the invoking module. ┃ lib/mindwendel_web/plugs/set_session_device_id.ex:11:9 #(Mindwendel.Plugs.SetSessionUserId.call) ┃ [D] ↘ Nested modules could be aliased at the top of the invoking module. ┃ lib/mindwendel_web/plugs/set_session_device_id.ex:9:10 #(Mindwendel.Plugs.SetSessionUserId.call) ┃ [D] ↘ Nested modules could be aliased at the top of the invoking module. ┃ lib/mindwendel_web/controllers/static_page_controller.ex:11:10 #(MindwendelWeb.StaticPageController.home) ┃ [D] ↘ Nested modules could be aliased at the top of the invoking module. ┃ lib/mindwendel_web/controllers/brainstorming_controller.ex:8:7 #(MindwendelWeb.BrainstormingController.create) ┃ [D] ↘ Nested modules could be aliased at the top of the invoking module. ┃ lib/mindwendel_web/controllers/admin/brainstorming_controller.ex:37:23 #(MindwendelWeb.Admin.BrainstormingController.fetch_user) ┃ [D] ↘ Nested modules could be aliased at the top of the invoking module. ┃ test/support/data_case.ex:34:7 #(Mindwendel.DataCase) ┃ [D] ↘ Nested modules could be aliased at the top of the invoking module. ┃ test/support/data_case.ex:31:11 #(Mindwendel.DataCase) ┃ [D] ↘ Nested modules could be aliased at the top of the invoking module. ┃ test/support/conn_case.ex:38:7 #(MindwendelWeb.ConnCase) ┃ [D] ↘ Nested modules could be aliased at the top of the invoking module. ┃ test/support/conn_case.ex:35:11 #(MindwendelWeb.ConnCase) ┃ [D] ↘ Nested modules could be aliased at the top of the invoking module. ┃ test/support/channel_case.ex:35:7 #(MindwendelWeb.ChannelCase) ┃ [D] ↘ Nested modules could be aliased at the top of the invoking module. ┃ test/support/channel_case.ex:32:11 #(MindwendelWeb.ChannelCase) ┃ [D] ↘ Nested modules could be aliased at the top of the invoking module. ┃ test/mindwendel_web/controllers/static_page_controller_test.exs:53:11 #(MindwendelWeb.StaticPageControllerTest) ┃ [D] ↘ Nested modules could be aliased at the top of the invoking module. ┃ test/mindwendel_web/controllers/static_page_controller_test.exs:38:11 #(MindwendelWeb.StaticPageControllerTest) ┃ [D] ↘ Nested modules could be aliased at the top of the invoking module. ┃ test/mindwendel_web/controllers/static_page_controller_test.exs:15:14 #(MindwendelWeb.StaticPageControllerTest) Code Readability ┃ ┃ [R] ↘ Do not use parentheses when defining a function which has no arguments. ┃ lib/mindwendel_web/plugs/set_response_header_content_security_policy.ex:78:7 #(Mindwendel.Plugs.SetResponseHeaderContentSecurityPolicy.get_websocket_scheme) ┃ [R] ↘ Line is too long (max is 120, was 123). ┃ lib/mindwendel_web/live/brainstorming_live/show.ex:86:121 #(MindwendelWeb.BrainstormingLive.Show.handle_info) ┃ [R] ↘ Line is too long (max is 120, was 135). ┃ lib/mindwendel_web/live/brainstorming_live/show.ex:63:121 #(MindwendelWeb.BrainstormingLive.Show.handle_info) ┃ [R] ↘ Line is too long (max is 120, was 128). ┃ lib/mindwendel/accounts.ex:152:121 #(Mindwendel.Accounts.user_has_active_brainstormings) ┃ [R] ↘ Line is too long (max is 120, was 204). ┃ lib/mindwendel/accounts.ex:133:121 #(Mindwendel.Accounts.delete_inactive_users) ┃ [R] ↘ The alias `Mindwendel.Repo` is not alphabetically ordered among its group. ┃ test/support/factory.ex:2:9 #(Mindwendel.Factory) ┃ [R] ↘ The alias `Mindwendel.Factory` is not alphabetically ordered among its group. ┃ test/mindwendel_web/live/brainstorming_live_test.exs:6:9 #(MindwendelWeb.BrainstormingLiveTest) ┃ [R] ↘ Do not use parentheses when defining a function which has no arguments. ┃ lib/mindwendel_web/plugs/set_response_header_content_security_policy.ex:71:7 #(Mindwendel.Plugs.SetResponseHeaderContentSecurityPolicy.get_port) ┃ [R] ↘ Do not use parentheses when defining a function which has no arguments. ┃ lib/mindwendel_web/plugs/set_response_header_content_security_policy.ex:64:7 #(Mindwendel.Plugs.SetResponseHeaderContentSecurityPolicy.get_scheme) ┃ [R] ↘ Do not use parentheses when defining a function which has no arguments. ┃ lib/mindwendel_web/plugs/set_response_header_content_security_policy.ex:58:7 #(Mindwendel.Plugs.SetResponseHeaderContentSecurityPolicy.get_style_src) ┃ [R] ↘ Do not use parentheses when defining a function which has no arguments. ┃ lib/mindwendel_web/plugs/set_response_header_content_security_policy.ex:52:7 #(Mindwendel.Plugs.SetResponseHeaderContentSecurityPolicy.get_script_src) ┃ [R] ↘ Do not use parentheses when defining a function which has no arguments. ┃ lib/mindwendel_web/plugs/set_response_header_content_security_policy.ex:45:7 #(Mindwendel.Plugs.SetResponseHeaderContentSecurityPolicy.get_host) ┃ [R] ↘ Line is too long (max is 120, was 149). ┃ lib/mindwendel_web/plugs/set_response_header_content_security_policy.ex:28:121 #(Mindwendel.Plugs.SetResponseHeaderContentSecurityPolicy.content_security_policy_directives) ┃ [R] ↘ Do not use parentheses when defining a function which has no arguments. ┃ lib/mindwendel_web/plugs/set_response_header_content_security_policy.ex:18:7 #(Mindwendel.Plugs.SetResponseHeaderContentSecurityPolicy.content_security_policy_directives) ┃ [R] ↘ The alias `Mindwendel.Ideas` is not alphabetically ordered among its group. ┃ lib/mindwendel_web/live/idea_live/index_component.ex:5:9 #(MindwendelWeb.IdeaLive.IndexComponent) ┃ [R] ↘ The alias `Mindwendel.Ideas` is not alphabetically ordered among its group. ┃ lib/mindwendel_web/live/brainstorming_live/show.ex:6:9 #(MindwendelWeb.BrainstormingLive.Show) ┃ [R] ↘ The alias `Mindwendel.Brainstormings.IdeaLabelFactory` is not alphabetically ordered among its group. ┃ lib/mindwendel_web/live/admin/brainstorming_live/edit.ex:4:9 #(MindwendelWeb.Admin.BrainstormingLive.Edit) ┃ [R] ↘ Line is too long (max is 120, was 122). ┃ lib/mindwendel/likes.ex:59:121 #(Mindwendel.Likes.delete_like) ┃ [R] ↘ The alias `Mindwendel.Brainstormings.Like` is not alphabetically ordered among its group. ┃ lib/mindwendel/ideas.ex:10:9 #(Mindwendel.Ideas) ┃ [R] ↘ The alias `Mindwendel.Brainstormings.IdeaLabel` is not alphabetically ordered among its group. ┃ lib/mindwendel/brainstormings/idea.ex:6:9 #(Mindwendel.Brainstormings.Idea) ┃ [R] ↘ Line is too long (max is 120, was 125). ┃ lib/mindwendel/brainstormings/brainstorming.ex:18:121 #(Mindwendel.Brainstormings.Brainstorming) ┃ [R] ↘ The alias `Mindwendel.Brainstormings.IdeaLabel` is not alphabetically ordered among its group. ┃ lib/mindwendel/brainstormings/brainstorming.ex:7:9 #(Mindwendel.Brainstormings.Brainstorming) ┃ [R] ↘ The alias `Mindwendel.Brainstormings.Idea` is not alphabetically ordered among its group. ┃ lib/mindwendel/brainstormings.ex:9:9 #(Mindwendel.Brainstormings) ┃ [R] ↘ The alias `Mindwendel.Repo` is not alphabetically ordered among its group. ┃ lib/mindwendel/accounts.ex:3:9 #(Mindwendel.Accounts) ┃ [R] ↘ The alias `Mindwendel.Repo` is not alphabetically ordered among its group. ┃ test/repo/data_migrations/migrate_idea_labels_to_idea_idea_labels_test.exs:7:9 #(Mindwendel.Repo.DataMigrations.MigrateIdeaLabelsToIdeaIdeaLabelsTest) ┃ [R] ↘ The alias `Mindwendel.Repo` is not alphabetically ordered among its group. ┃ test/repo/data_migrations/migrate_idea_labels_test.exs:6:9 #(Mindwendel.Repo.DataMigrations.MigrateIdealLabelsTest) ┃ [R] ↘ The alias `Mindwendel.Brainstormings` is not alphabetically ordered among its group. ┃ test/mindwendel_web/live/brainstorming_live/show_idea_edit_test.exs:4:9 #(MindwendelWeb.BrainstormingLive.ShowIdeaEditTest) ┃ [R] ↘ The alias `Mindwendel.Repo` is not alphabetically ordered among its group. ┃ test/mindwendel_web/controllers/brainstorming_controller_test.exs:6:9 #(MindwendelWeb.BrainstormingControllerTest) ┃ [R] ↘ The alias `Mindwendel.Factory` is not alphabetically ordered among its group. ┃ test/mindwendel_web/channels/brainstorming_channel_test.exs:4:9 #(MindwendelWeb.BrainstormingChannelTest) ┃ [R] ↘ The alias `Mindwendel.Ideas` is not alphabetically ordered among its group. ┃ test/mindwendel/ideas_test.exs:5:9 #(Mindwendel.IdeasTest) ┃ [R] ↘ The alias `Mindwendel.Ideas` is not alphabetically ordered among its group. ┃ test/mindwendel/idea_test.exs:5:9 #(Mindwendel.IdeaTest) ┃ [R] ↘ The alias `Mindwendel.Ideas` is not alphabetically ordered among its group. ┃ test/mindwendel/idea_labels_test.exs:5:9 #(Mindwendel.IdeaLabelsTest) ┃ [R] ↘ The alias `Mindwendel.Factory` is not alphabetically ordered among its group. ┃ test/mindwendel/csv_formatter_test.exs:3:9 #(MindwendelServices.CSVFormatter) ┃ [R] ↘ The alias `Mindwendel.Factory` is not alphabetically ordered among its group. ┃ test/mindwendel/brainstormings_users_test.exs:3:9 #(Mindwendel.AccountsMergeBrainstormingUserTest) ┃ [R] ↘ The alias `Mindwendel.IdeaLabels` is not alphabetically ordered among its group. ┃ test/mindwendel/brainstormings_test.exs:8:9 #(Mindwendel.BrainstormingsTest) ┃ [R] ↘ The alias `Mindwendel.Brainstormings.IdeaLabel` is not alphabetically ordered among its group. ┃ test/mindwendel/brainstormings/create_brainstorming_test.exs:9:9 #(Mindwendel.Brainstormings.CreateBrainstormingTest) ┃ [R] ↘ The alias `Mindwendel.Brainstormings.BrainstormingModeratingUser` is not alphabetically ordered among its group. ┃ test/mindwendel/brainstormings/create_brainstorming_test.exs:3:9 #(Mindwendel.Brainstormings.CreateBrainstormingTest) ┃ [R] ↘ Line is too long (max is 120, was 138). ┃ test/mindwendel/accounts_test.exs:127:121 #(Mindwendel.AccountsTest) ┃ [R] ↘ Line is too long (max is 120, was 138). ┃ test/mindwendel/accounts_test.exs:109:121 #(Mindwendel.AccountsTest) ┃ [R] ↘ Line is too long (max is 120, was 138). ┃ test/mindwendel/accounts_test.exs:91:121 #(Mindwendel.AccountsTest) ┃ [R] ↘ The alias `Mindwendel.Factory` is not alphabetically ordered among its group. ┃ test/mindwendel/accounts_test.exs:3:9 #(Mindwendel.AccountsTest) ┃ [R] ↘ Do not use parentheses when defining a function which has no arguments. ┃ lib/mindwendel_web/views/static_page.ex:9:7 #(MindwendelWeb.StaticPageView.brainstormings_available_until) ┃ [R] ↘ Line is too long (max is 120, was 124). ┃ lib/mindwendel_web/router.ex:25:121 #(MindwendelWeb.Router) ┃ [R] ↘ Line is too long (max is 120, was 122). ┃ lib/mindwendel_web/router.ex:23:121 #(MindwendelWeb.Router) ┃ [R] ↘ Line is too long (max is 120, was 127). ┃ lib/mindwendel_web/router.ex:21:121 #(MindwendelWeb.Router) ┃ [R] ↘ Line is too long (max is 120, was 162). ┃ lib/mindwendel_web/router.ex:20:121 #(MindwendelWeb.Router) ┃ [R] ↘ The alias `Mindwendel.Brainstormings` is not alphabetically ordered among its group. ┃ lib/mindwendel_web/controllers/brainstorming_controller.ex:3:9 #(MindwendelWeb.BrainstormingController) ┃ [R] ↘ The alias `Mindwendel.Ideas` is not alphabetically ordered among its group. ┃ lib/mindwendel_web/controllers/admin/brainstorming_controller.ex:4:9 #(MindwendelWeb.Admin.BrainstormingController) ┃ [R] ↘ The alias `Mindwendel.Brainstormings` is not alphabetically ordered among its group. ┃ lib/mindwendel/worker/remove_brainstormings_and_users_after_period_worker.ex:3:9 #(Mindwendel.Worker.RemoveBrainstormingsAndUsersAfterPeriodWorker) ┃ [R] ↘ The alias `Mindwendel.Ideas` is not alphabetically ordered among its group. ┃ lib/mindwendel/likes.ex:10:9 #(Mindwendel.Likes) ┃ [R] ↘ The alias `Mindwendel.Brainstormings.Idea` is not alphabetically ordered among its group. ┃ lib/mindwendel/idea_labels.ex:9:9 #(Mindwendel.IdeaLabels) ┃ [R] ↘ The alias `Mindwendel.Repo` is not alphabetically ordered among its group. ┃ lib/mindwendel/help.ex:7:9 #(Mindwendel.Help) ┃ [R] ↘ The alias `Mindwendel.Brainstormings.Idea` is not alphabetically ordered among its group. ┃ lib/mindwendel/brainstormings/like.ex:5:9 #(Mindwendel.Brainstormings.Like) ┃ [R] ↘ The alias `Mindwendel.Brainstormings.Brainstorming` is not alphabetically ordered among its group. ┃ lib/mindwendel/brainstormings/brainstorming_moderating_user.ex:3:9 #(Mindwendel.Brainstormings.BrainstormingModeratingUser) ┃ [R] ↘ The alias `Mindwendel.Brainstormings.Idea` is not alphabetically ordered among its group. ┃ lib/mindwendel/accounts/user.ex:7:9 #(Mindwendel.Accounts.User) ┃ [R] ↘ The alias `Mindwendel.Brainstormings.Brainstorming` is not alphabetically ordered among its group. ┃ lib/mindwendel/accounts/brainstorming_user.ex:3:9 #(Mindwendel.Accounts.BrainstormingUser) ```Feature Ideas
<Ctrl> + <Enter>
would be awesomeDependencies
Switching libraries
Switching libraries usually has little benefit but I still wanted to mention some
Tests
One of my todos to look at. First look we don't seem to have a full integration test?
CI