Sanem sarioglu/feature/subtopic listings

[SanemSarioglu]

What I did

• Implemented sub-skill support on the New Skill screen:
• Populate sub-skill options when a MainSubject is selected.
• Show a sub-skill dropdown only when a subject is selected.
• Make sub-skill required when a subject is chosen (validation message shown on Save).
• Added/updated unit and UI tests covering sub-skill visibility, selection and validation.

How I did it

• Goal: make sub-skill a contextual, required field only when a main subject is chosen and keep UI/state consistent when the subject changes.
• Approach:
  1. When a subject is selected I load the corresponding sub-skill names via SkillsHelper.getSkillNames(subject), update subSkillOptions, reset selectedSubSkill and clear any subject/sub-skill errors so the UI shows a fresh sub-skill list for the new subject.
  2. I added setSubSkill(subSkill: String) to store the chosen sub-skill and clear its validation error immediately when a choice is made.
  3. Validation: I made sub-skill required only when a subject is present. The generic setError() now sets invalidSubSkillMsg only if subject != null and selectedSubSkill is missing — this ensures users who haven't chosen a subject aren't shown a sub-skill error.
  4. UI: the sub-skill dropdown is shown conditionally when a subject is set, and it reads options from the subSkillOptions list to avoid stale/incorrect choices after subject changes.
  5. Tests: I added/adjusted unit and UI tests to verify sub-skill visibility, options refresh on subject change, selection clears errors, and validation prevents saving without a sub-skill when a subject is selected.

How to verify it

1. Open New Skill screen, confirm sub-skill field is hidden initially.
2. Select a Subject → sub-skill dropdown appears and is populated.
3. Attempt Save without selecting sub-skill → sub-skill error appears.
4. Select a sub-skill → error clears.

Demo video

Pre-merge checklist

The changes I introduced:

• work correctly
• do not break other functionalities
• work correctly on Android
• are fully tested (or have tests added)

[NedenSinir]

Overall good, small changes can be considered. No sonarcloude issue nor CI

[NedenSinir]

Summary: overall changes look reasonable. I found two real test bugs (missing assertions) and a couple of small improvements/safety tweaks you may want to apply

• Problem: Two tests evaluate fetchSemanticsNodes().isNotEmpty() but do not assert the result — the boolean return value is unused, so the tests won't fail if the element isn't present. Fix by asserting (assertTrue or using compose assertions).
• Minor: tests rely on collection indexing ([0]) — OK but can be flaky if dropdown order changes.

Suggested fix (replace the two test bodies to assert the presence):

// showsError_whenNoSubject_onSave
@Test
fun showsError_whenNoSubject_onSave() {
  // Ensure subject is empty (initial screen state), click Save
  compose.onNodeWithTag(NewSkillScreenTestTag.BUTTON_SAVE_SKILL).performClick()

  // Error helper under Subject field should be visible
  val nodes = compose
      .onAllNodesWithTag(NewSkillScreenTestTag.INVALID_SUBJECT_MSG, useUnmergedTree = true)
      .fetchSemanticsNodes()
  assertTrue("Expected invalid subject message to be present", nodes.isNotEmpty())
}

// showsError_whenSubjectChosen_butNoSubSkill_onSave
@Test
fun showsError_whenSubjectChosen_butNoSubSkill_onSave() {
  // Choose a subject
  compose.onNodeWithTag(NewSkillScreenTestTag.SUBJECT_FIELD).performClick()
  compose.onAllNodesWithTag(NewSkillScreenTestTag.SUBJECT_DROPDOWN_ITEM_PREFIX)[0].performClick()

  // Sub-skill field visible now but we don't choose any sub-skill
  // Click Save directly
  compose.onNodeWithTag(NewSkillScreenTestTag.BUTTON_SAVE_SKILL).performClick()

  // Error helper under Sub-skill field should be visible
  val nodes = compose
      .onAllNodesWithTag(NewSkillScreenTestTag.INVALID_SUB_SKILL_MSG, useUnmergedTree = true)
      .fetchSemanticsNodes()
  assertTrue("Expected invalid sub-skill message to be present", nodes.isNotEmpty())
}

[SanemSarioglu]

Done

[NedenSinir]

Looks good to me

[NedenSinir]

Mostly good. A couple of small suggestions:

• You're force-unwrapping selectedSubSkill with !! in addSkill. isValid guards this, but it's safer to avoid !! and bail early if something unexpected occurs.
• Consider returning early or logging instead of !! to make code more robust.

if (state.isValid) {
  val price = state.price.toDouble()
  val specificSkill = state.selectedSubSkill ?: return // safer than !!
  val newSkill =
      Skill(
          mainSubject = state.subject!!,
          skill = specificSkill,
      )
  ...
}

-And maybe for error string not just "Error" but you can use "network error" to make it more descriptive

[SanemSarioglu]

Done

[NedenSinir]

Tests look comprehensive and updated to include sub-skill flows.
Minor suggestions:

• The FakeLocationRepo comment and signature were simplified; ensure the test file still compiles with the chosen constructor signature.
• Consider keeping more descriptive exception messages in fake repos (e.g., "Network error") to aid debugging when a test fails.

[sonarqubecloud]

Quality Gate failed

Failed conditions
67.3% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

[NedenSinir]

Looks all good now