Skip to content

5169 Rework of Getting Started section #5229

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Budmin
Copy link

@Budmin Budmin commented Jun 7, 2025

Resolves #5169

Description

  • Changed "Getting Started" section to provide links to all resources in one section.
  • Added "bank_is_set_up" flag to the organization model to dictate whether the section is visible.
  • This was done to ensure that the "Getting Started" section remains available until the user is finished with it.
  • While this is a slightly less interactive workflow, it's more useful to the user.

Type of change

(Calling this a feature is a bit much, but it is technically changing functionality.)

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Documentation update

How Has This Been Tested?

As there was already a test that verified the previous "Getting Started" section. This test was modified to test for the new workflow. No configuration changes should be necessary to run this updated test.

@Budmin
Copy link
Author

Budmin commented Jun 7, 2025

If it's preferable for me to resolve these conflicts I am willing to do so.
Whatever is best for the project.

@cielf
Copy link
Collaborator

cielf commented Jun 7, 2025

They look pretty straight forward to me - please go ahead.

@Budmin
Copy link
Author

Budmin commented Jun 8, 2025

@cielf If there's anything I need to change, please let me know!

Copy link
Collaborator

@cielf cielf left a comment

Choose a reason for hiding this comment

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

Hey @Budmin -- thank you for this ! i think it's going to be more intuitive for the new banks.

Just to let you know our process -- I do the functional review, then, once everything seems to work fine, I pass it on to a senior dev for technical comments.

There's 1 thing that has to be changed, plus a couple of nice to haves that became evident when I tried it out.

Needed:
1/ When you click "Dismiss", it's currently going to the organization -- it should stay on the dashboard.

Desired:
Most of the user tasks are pretty straight-forward, but a couple of them will be easier if the user reads the appropriate user guide section. Let's make that easy for them.

1/ beside "Audits", add a link titled: (User guide) pointing to https://rubyforgood.github.io/human-essentials/user_guide/bank/getting_started_inventory.html

2/ beside "Customize your organization", add a link titled (User guide) pointing to https://rubyforgood.github.io/human-essentials/user_guide/bank/getting_started_customization.html

3/ This is a very small semantic difference, but I think we should change "Add custom items" to "Add any custom items".

Your changes to the user guide look good. We'll need to revise the entire "Getting started" section to make it flow more like this, but I think that's out of scope of this PR.

Thank you.

@@ -838,5 +838,4 @@ DEPENDENCIES
web-console
webmock (~> 3.25)

BUNDLED WITH
2.6.7
2.6.8
Copy link

Choose a reason for hiding this comment

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

unintentional upgrade?

@compwron compwron requested a review from Copilot June 9, 2025 20:33
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR reworks the "Getting Started" user workflow by consolidating resource links into one section and introducing a new flag (bank_is_set_up) to track when an organization has completed the setup. Key changes include updating tests, modifying documentation links and text, and revising view templates and controller strong parameters to incorporate the new bank_is_set_up flag.

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
spec/system/dashboard_system_spec.rb Updated functional test to check guide visibility based on bank_is_set_up flag
spec/models/organization_spec.rb and spec/factories/organizations.rb Updated documentation comments for new bank_is_set_up column
docs/user_guide/bank/* Modified text and image links in getting started user guide
db/schema.rb and db/migrate/20250513012435_add_bank_is_set_up.rb Added bank_is_set_up column with migration and schema changes
app/views/dashboard/index.html.erb Changed partial render locals to use bank_is_set_up flag
app/views/dashboard/_getting_started_prompt.html.erb Reworked guide display logic and added a dismiss button
app/controllers/organizations_controller.rb, dashboard_controller.rb, admin/organizations_controller.rb Updated strong parameters and instance variable assignment
Comments suppressed due to low confidence (1)

app/controllers/dashboard_controller.rb:6

  • For clarity and consistency with the model attribute, consider renaming @org_is_set_up to @bank_is_set_up.
 @org_is_set_up = current_organization.bank_is_set_up

</div>
</div>
</div>

<p>When you're done, click this box to hide this section</p>
<%= button_to "Dismiss", "/manage", method: :patch, params: {organization: { bank_is_set_up: true }}, class: "btn btn-primary" %>
Copy link
Preview

Copilot AI Jun 9, 2025

Choose a reason for hiding this comment

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

Consider using a route helper in place of hardcoding the '/manage' path to improve maintainability and clarity of intent.

Suggested change
<%= button_to "Dismiss", "/manage", method: :patch, params: {organization: { bank_is_set_up: true }}, class: "btn btn-primary" %>
<%= button_to "Dismiss", manage_path, method: :patch, params: {organization: { bank_is_set_up: true }}, class: "btn btn-primary" %>

Copilot uses AI. Check for mistakes.

@compwron
Copy link

compwron commented Jun 9, 2025

I let CI start running
I am reading the github issue to understand better the goal :) Sorry if I ask beginner questions...

@compwron compwron changed the title 5169 getting started rework 5169 Rework of Getting Started section Jun 9, 2025
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.

Rework of Getting Started section
3 participants