Skip to content

level_group levels - fix issue with page numbers popping up in url#65192

Merged
kobryan0619 merged 1 commit into
stagingfrom
kaitie/investigate-level-group-levels
Apr 11, 2025
Merged

level_group levels - fix issue with page numbers popping up in url#65192
kobryan0619 merged 1 commit into
stagingfrom
kaitie/investigate-level-group-levels

Conversation

@kobryan0619

@kobryan0619 kobryan0619 commented Apr 10, 2025

Copy link
Copy Markdown
Contributor

This PR solves 2 problems with one line of code.

Issue 1: For level_group levels that were marked as an "assessment" at a script_level, we were getting some weird behavior with the URL automatically adding a page. This was causing issues with teachers being able to access the summary of student response's. This was because the combination of being marked as an assessment and a LevelGroup triggered actions to treat this as a long_assessment (which included page numbers). The summary button just appended summary to the "paginated" URL which was causing the issues for the user.

image

Action 1: Modified how long_assessment is defined in the script_level model to avoid these activity_guide_levels from being included in long_assessment. I confirmed with Ken that all activity_guide_levels are intended to be one page.


image

Issue 2: We also noticed an issue with how bubbles were showing up for these types of levels (level_group levels marked as assessments. The bubble wasn't getting big and showing the number when selected. This also stemmed from the long_assessment designation which caused a puzzle_page to be assigned to the level in redux through the controller, some haml - tracing this code got a little wonky (to maybe hand-wavy), but I am pretty sure this is where the problems stemmed from.

Action 2: Again, removing the long_assessment designation meant we were no longer assigning a page which fixed the issue where the current page number in redux wasn't matching the level's page_number

Links

Thread
Other thread

Testing story

Tested locally

@sanchitmalhotra126 sanchitmalhotra126 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix! I don't have a ton of context here but as explained, this makes sense to me. A few questions for my understanding:

  • What is an activity_guide_level?
  • Is there an implication here that for level groups that are not marked activity_guide_level, we don't want to be able to view summaries? (because presumably the /summary path will still be broken?)
  • [edit] also, is there an implication that activity_guide_level level groups will always be a single page?

@davidsbailey davidsbailey left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice sleuthing Kaitie and Ken! Seems like a good quick fix!

some new UI test coverage seems appropriate since these are end-to-end issues. Also it seems like the error may have snuck under the radar in part because the allthethings level is not marked as an assessment but the pswai-pilot-2024 level is. based on that I would suggest adding the following to level_group_activity_guide.feature:

  1. can see the current level number in the progress bar
  2. can click summary button and the page renders at least one student response
  3. testing the above with and without the level being marked as an assessment

I'm not totally sure I understand when we do / don't want level groups to be assessment levels, so there could be some flexibility on 3, but unless we can convince ourselves otherwise that these will always (or never) be assessments, testing both scenarios seems safest to me.

Lastly it could be worth (manually) testing what happens if you add multiple pages to one of these activity guide levels, just to make sure it doesn't fail in some confusing way.

OK to merge as-is to unblock our users, but I would strongly encourage a follow-up PR to add some UI tests before returning to other tasks.

@kobryan0619

kobryan0619 commented Apr 11, 2025

Copy link
Copy Markdown
Contributor Author

@sanchitmalhotra126

What is an activity_guide_level?

These are a newer use of a DSL level (PR). It is intended to work similar to how our survey or "test" levels work, with a few slight modifications.

Is there an implication here that for level groups that are not marked activity_guide_level, we don't want to be able to view summaries? (because presumably the /summary path will still be broken?)

Right now, we only show the summary button on level_group levels marked as activity_guides. That button should not be shown on other multi-page assessments.

[edit] also, is there an implication that activity_guide_level level groups will always be a single page?

According to Ken, as a curriculum team member, yes. That being said, I think we can put some safeguards in place to make sure a creative-problem-solving-curriculum-team-member doesn't break this norm (until we can decide if we want to support this use case or not). This was also part of a follow-up Dave suggested as well

@kobryan0619 kobryan0619 merged commit 4ddae9c into staging Apr 11, 2025
@kobryan0619 kobryan0619 deleted the kaitie/investigate-level-group-levels branch April 11, 2025 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants