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

feat(helpBubble): add Academy link and update text of existing links TASK-1466 #5437

Merged
merged 5 commits into from
Jan 28, 2025

Conversation

magicznyleszek
Copy link
Member

@magicznyleszek magicznyleszek commented Jan 22, 2025

📣 Summary

Add new link to KoboToolbox Academy in the HelpBubble popup. The link is configurable through /admin UI (constance). Two existing links in the same popup had their text updated.

💭 Notes

Before --> after:
Screenshot 2025-01-23 at 13 29 01

👀 Preview steps

Feature/no-change template:

  1. ℹ️ have account and be logged in
  2. open HelpBubble (down left area of UI, accessible on almost all routes)
  3. 🟢 notice that new element was added
  4. 🟢 notice that previous 2 elements have changed text

@magicznyleszek magicznyleszek marked this pull request as ready for review January 23, 2025 12:37
@magicznyleszek magicznyleszek requested a review from duvld January 24, 2025 09:23
Copy link

@@ -217,6 +217,12 @@
env.str('KOBO_SUPPORT_URL', 'https://support.kobotoolbox.org/'),
'URL for "KoboToolbox Help Center"',
),
'ACADEMY_URL': (
Copy link
Member

Choose a reason for hiding this comment

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

Curious why do we need to check the environment for these links?

Copy link
Contributor

Choose a reason for hiding this comment

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

I just followed the same code for the other URLs. But yeah, maybe it's not necessary for this one

@@ -316,7 +317,7 @@ def test_free_tier_override_uses_organization_owner_join_date(
def test_social_apps(self):
# GET mutates state, call it first to test num queries later
self.client.get(self.url, format='json')
queries = FuzzyInt(18, 27)
queries = FuzzyInt(18, 28)
Copy link
Member

Choose a reason for hiding this comment

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

Why this change? Not disagreeing with it but I'm just not sure where this is coming from

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this checks how many things are there in environment endpoint, and since we've added one, it needs to be increased:)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, a new constance configuration parameter brings an additional DB query

@magicznyleszek magicznyleszek merged commit 8dca2dd into main Jan 28, 2025
7 checks passed
@magicznyleszek magicznyleszek deleted the leszek/task-1466-academy-button branch January 28, 2025 23:03
@magicznyleszek magicznyleszek changed the title feat(helpBubble): add Academy link and update text of existing links feat(helpBubble): add Academy link and update text of existing links TASK-1466 Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants