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

NAS-131266 / 25.04 / Cancel button takes back to apps page #10802

Merged
merged 5 commits into from
Oct 10, 2024
Merged

Conversation

RehanY147
Copy link
Contributor

Changes:

When view button for container logs for an app is clicked, user is taken to show container logs. The cancel button previously didn't take them back to the apps page. Now it does.

Testing:

See ticket description for testing.

@RehanY147 RehanY147 requested a review from a team as a code owner October 3, 2024 19:42
@RehanY147 RehanY147 requested review from denysbutenko and removed request for a team October 3, 2024 19:42
@bugclerk bugclerk changed the title Cancel button takes back to apps page NAS-131266 / 25.04 / Cancel button takes back to apps page Oct 3, 2024
@bugclerk
Copy link
Contributor

bugclerk commented Oct 3, 2024

Copy link

codecov bot commented Oct 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.55%. Comparing base (303e5cc) to head (83fab99).
Report is 30 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10802      +/-   ##
==========================================
+ Coverage   80.91%   81.55%   +0.63%     
==========================================
  Files        1571     1579       +8     
  Lines       52114    54023    +1909     
  Branches     5812     5794      -18     
==========================================
+ Hits        42167    44056    +1889     
- Misses       9947     9967      +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@denysbutenko denysbutenko left a comment

Choose a reason for hiding this comment

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

It works well but tests need cleanup

timestamp: '[12:34]',
data: 'Some logs.',
describe('When dialog is set a value', () => {
const createComponent = createComponentFactory({
Copy link
Member

Choose a reason for hiding this comment

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

Let's reuse test definition and extract it into setupTest method that accepts params that needs to be changed

@RehanY147 RehanY147 merged commit 3c418ee into master Oct 10, 2024
6 checks passed
@RehanY147 RehanY147 deleted the NAS-131266 branch October 10, 2024 05:53
@bugclerk
Copy link
Contributor

This PR has been merged and conversations have been locked.
If you would like to discuss more about this issue please use our forums or raise a Jira ticket.

@truenas truenas locked as resolved and limited conversation to collaborators Oct 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants