Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix to new badge being shown on channel delete #11676

Conversation

thesujai
Copy link
Contributor

Summary

The cause of the bug is well addressed in #6730 (comment)

For handling that we will show new badge based on the latest task which will be the task with highest scheduled_datetime(among the tasks with same channel_id)
Screencast
Here I delete resource initially(shows no new badge)
Then install more resource(shows new badge as it should)
Then remove a resource(shows no new badge, as it shouldn't)
Screencast from 28-12-23 04:11:18 PM IST.webm

References

Fixes #6730

Reviewer guidance


Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@github-actions github-actions bot added APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) DEV: frontend SIZE: small labels Dec 28, 2023
@thesujai thesujai changed the title added isLatestCheck Fix to new badge being shown on channel delete Dec 28, 2023
Copy link
Member

@rtibbles rtibbles left a comment

Choose a reason for hiding this comment

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

Code changes look fine - @pcenov could you verify the fix? It seems that the key to replication is to have imported some resources, and then to have deleted a resource without having cleared the import task.

@rtibbles
Copy link
Member

rtibbles commented Jan 2, 2024

@thesujai this is great, could you retarget to release-v0.16.x and rebase to that branch too? We can release this in a patch release to 0.16.

@rtibbles rtibbles added this to the 0.16 Planned Patch 1 milestone Jan 2, 2024
@thesujai thesujai changed the base branch from develop to release-v0.16.x January 2, 2024 16:27
@github-actions github-actions bot added DEV: dev-ops Continuous integration & deployment DEV: renderers HTML5 apps, videos, exercises, etc. DEV: backend Python, databases, networking, filesystem... APP: Facility Re: Facility App (user/class management, facility settings, csv import/export, etc.) APP: Learn Re: Learn App (content, quizzes, lessons, etc.) APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) APP: User Re: User app (sign-in, sign-up, user profile, etc.) APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) DEV: tools Internal tooling for development SIZE: medium labels Jan 2, 2024
@thesujai thesujai force-pushed the new-badge-on-delete-channel branch from 7b94dda to 7941f44 Compare January 2, 2024 16:45
@thesujai
Copy link
Contributor Author

thesujai commented Jan 2, 2024

Hey @rtibbles 👋🏽
🚀 Changes are now targeted for release-v0.16.x and rebased onto it. Ready to roll for a patch release to 0.16!
Check it out and let me know if tweaks are needed.

@rtibbles rtibbles removed DEV: dev-ops Continuous integration & deployment DEV: renderers HTML5 apps, videos, exercises, etc. DEV: backend Python, databases, networking, filesystem... APP: Facility Re: Facility App (user/class management, facility settings, csv import/export, etc.) APP: Learn Re: Learn App (content, quizzes, lessons, etc.) APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) APP: User Re: User app (sign-in, sign-up, user profile, etc.) APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) labels Jan 2, 2024
@rtibbles rtibbles removed DEV: tools Internal tooling for development SIZE: medium labels Jan 2, 2024
@pcenov
Copy link
Member

pcenov commented Jan 3, 2024

@rtibbles I confirm that this is implemented as specified, passes manual QA. Thank you @thesujai!

@rtibbles
Copy link
Member

rtibbles commented Jan 3, 2024

Thanks @thesujai - looks like this is ready to go. We'll hold off merging until 0.16.0 has been released, but no more work needs to be done here!

Copy link
Member

@rtibbles rtibbles left a comment

Choose a reason for hiding this comment

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

Actually, we changed our minds, and this is safe enough to merge for 0.16.0, thank you for your contribution!

@rtibbles rtibbles merged commit 528b3c8 into learningequality:release-v0.16.x Jan 8, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) DEV: frontend SIZE: small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New icon is shown in channel when a resource was deleted.
3 participants