Separate mod importing business logic from the UI component

[VilppeRiskidev]

No description provided.

[VilppeRiskidev]

Moved a couple of functions away from the ui component, still more stuff to do. You can review the progress to check if the approach is OK.

[anttimaki]

• It was discussed that this should be one commit (or PR) per moved method so I'm disappointed to see these changed lumped together. Not a blocker though.
• Blocker: moving the business logic to ProfilesMixin.vue still means they're in a vue component. The point of the task was to not have these in vue components anymore, but rather on Vuex or separate utils files, depending on how much they depend on the Vuex state
• Blocker: at least on my local env, importing a profile no longer shows the progress modal. Everything happens on the background. Use can change the selected profile while import is in progress
• Blocker: some of the new methods are missing types for arguments
• Remember to clean out unused imports when moving code out of files (well, always when making changes, I guess)
• Having the createProfileClicked methods receive the profile name as an argument seems a bit confusing when all other places access it through this.newProfileName

[VilppeRiskidev]

Didn't test/review my changes very deeply yet, but shouldn't be any big bugs (probably jinxing myself).
Quite very complicated logic, so many functions at a time, but it's still one logical set (everything that's refactored is executed by a single user action). The rest of the functions/logic will likely be refactored more like one function at a time.

[anttimaki]

@VilppeRiskidev

So this is sort of an intuition thing which might be partly triggered by the all-things-at-once approach, but I don't think this is the way we want to refactor this either. Some more concrete things:

• Having to pass the modal component itself as a function parameter to helper indicates that there is no true separation, it just changes it so now the business logic depends on the UI component instead of vice versa.
  • Also there's sort of circular import going on now
• Passing component methods that wrap Vuex methods kinda indicates the same thing
  • I think it should be possible to call Vuex store directly from the helper if we need to, so we don't have pass these wrappers around. But it would be cleaner if we didn't have to resort to that at all. Most likely it would be better to move such methods to Vuex, or split them to parts that access Vuex from the Vue component and does the rest in utils

---

Generally speaking, as a rule of thumb I've liked to have component methods to be something like this when feasible:

try {
    ProfileUtils.doOneThing();
    // Possible more ProfileUtils calls if they share error handling
} catch (e) {
    this.$store.commit('error/handleError', R2Error.fromThrownValue(e));
}

Also generally speaking each helper function should take a limited number of "simple" parameters (string, ThunderstoreMod) and return "simple" values. Callback functions make sometimes sense but personally I'd avoid other functions if it can be done easily.

---

Anyway, I agree that this component is a bit complex one to be the first exercise. I would suggest we abandon this branch and start from the scratch. We'd start from the easy cases the to get the ball rolling and worry about the more complex stuff later. Sometimes cleaning the easy stuff away reduces the complexity and makes the rest easier. Even if it doesn't, we can then think about the hard parts separately, which is cognitively easier than trying to handle everything at once. On a concrete level my suggestion would be:

• Create a separate standalone PR for moving one function/part of code
  • Test changes thoroughly to catch any bugs early so it's easier to see what change broke them
• Target to have one such PR daily, i.e. don't create more even if you could
• Have each PR merged before starting the work on the next one to avoid continuously rebasing stuff and to get feedback early
• This likely means you won't always work all day on this task. Work on other tasks that don't touch the import modal in paraller

If you agree with the above, I'd suggest the first three PRs would be:

1. Move extractZippedProfileFile to ProfileUtils
2. Move parseYamlToExportFormat to ProfileUtils
   • Consider renaming read argument to yamlContent oslt for clarity
3. Move installModAfterDownload to ProfileUtils
   • add profile argument to separate the tie to component (this.activeProfile)
   • Change return value to be only ManifestV2. If errors are encountered, throw the R2Error
     • Adjust downloadCompletedCallback from if to try-catch to compensate

We'll figure out the next steps when that time comes.

[anttimaki]

It was decided to redo this in smaller parts.