Skip to content

Conversation

@jelaniwoods
Copy link
Contributor

@jelaniwoods jelaniwoods commented Jun 6, 2021

Resolves #78
Resolves #88

Problem

Students often see a big ol' error page if Git or Gitpod settings aren't exactly right

image

Students should never see an error page like this. web_git should fail gracefully and redirect with a flash message if something doesn't work.

Solution

This PR does the following:

  • Enables session in Sinatra to display flash messages.
  • Refactors controller logic to rescue Git::GitExecuteError exceptions
  • Displays flash messages on redirect

web-git-alert

@jelaniwoods jelaniwoods changed the base branch from spring2020 to jw-enable-tooltips June 6, 2021 03:51
* rescue errors from git gem

* display and clear flash messages
@jelaniwoods jelaniwoods force-pushed the 78-jw-add-notice-and-alerts branch from 7ce1147 to 96f6962 Compare June 6, 2021 18:54
@jelaniwoods jelaniwoods marked this pull request as ready for review June 7, 2021 14:20
@jelaniwoods jelaniwoods requested a review from raghubetina June 7, 2021 20:11
@git ||= Git.open(working_dir)
end

def safe_git_action(method, **options)
Copy link
Contributor

Choose a reason for hiding this comment

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

Love this method/refactor.

@raghubetina
Copy link
Contributor

Great idea. LGTM 🚢

@jelaniwoods jelaniwoods merged commit 99d1dc2 into jw-enable-tooltips Jun 7, 2021
jelaniwoods added a commit that referenced this pull request Jun 7, 2021
Users should never see an error page if a Git action fails. web_git should fail gracefully and redirect with a flash message if something doesn't work.

This commit adds the following:
- enables session in Sinatra to display flash messages.
- refactors controller logic to rescue Git::GitExecuteError exceptions
- displays flash messages on redirect
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