Test coderabbit #8580
Test coderabbit #8580
Conversation
This sanitizes and decodes arguments to unicode that are passed in web requests to the server
Also, added logging for developers for all web routes when they have parameters defined in the method signature when they shouldn't be. Each method should explicitly use get_*_argument(s) methods to ensure passed data is safe and sanitized, devoid of control characters, injection safe, and formatted in the correct type. All route methods should only have `self` as a parameter, suck as `def searchIndexersForShowName(self): lang = self.get_argument("lang", strip=True)`
Signed-off-by: miigotu <[email protected]>
…ows.py get_*_argument(s) methods return body/query arguments percent-decoded, decoded to unicode, and sanitized with control characters and other unsafe characters removed.
Signed-off-by: miigotu <[email protected]>
This sanitizes and decodes arguments to unicode that are passed in web requests to the server
Also, added logging for developers for all web routes when they have parameters defined in the method signature when they shouldn't be. Each method should explicitly use get_*_argument(s) methods to ensure passed data is safe and sanitized, devoid of control characters, injection safe, and formatted in the correct type. All route methods should only have `self` as a parameter, suck as `def searchIndexersForShowName(self): lang = self.get_argument("lang", strip=True)`
…ows.py get_*_argument(s) methods return body/query arguments percent-decoded, decoded to unicode, and sanitized with control characters and other unsafe characters removed.
Signed-off-by: miigotu <[email protected]>
fix syntax checking for mako templates by changing quote type fix adding shows/existing keep global qualifier when using replaceAll make srPID consistent with scPID use target="_blank" rather than window.open use anon_url in a few more places avoid onclick in anchors, put javascript in core.js fix error with get_arguments where default is not a proper parameter fix formatting Signed-off-by: miigotu <[email protected]>
Signed-off-by: miigotu <[email protected]>
Signed-off-by: miigotu <[email protected]>
Signed-off-by: miigotu <[email protected]>
Updates the requirements on [flask](https://github.com/pallets/flask) to permit the latest version. - [Release notes](https://github.com/pallets/flask/releases) - [Changelog](https://github.com/pallets/flask/blob/main/CHANGES.rst) - [Commits](pallets/flask@2.2.2...3.0.0) --- updated-dependencies: - dependency-name: flask dependency-type: direct:development ... Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: miigotu <[email protected]>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
WalkthroughThis pull request includes a variety of changes across the codebase. It updates the configuration files, modifies the GitHub Actions workflows, and introduces changes to the SickChill application's Python and JavaScript code, as well as its HTML templates. The changes aim to improve the application's functionality, enhance its visual appearance, and streamline its CI/CD pipeline. Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 11
Files ignored due to filter (13)
- bower.json
- contrib/images/vector.svg
- package.json
- pyproject.toml
- sickchill/gui/slick/images/mattermost.png
- sickchill/gui/slick/images/network/abema tv.png
- sickchill/gui/slick/images/network/bilibili.png
- sickchill/gui/slick/images/network/itvx.png
- sickchill/gui/slick/images/network/tbs (jp).png
- sickchill/gui/slick/images/network/tubi.png
- sickchill/gui/slick/js/core.min.js
- sickchill/gui/slick/js/vendor.min.js
- yarn.lock
Files selected for processing (57)
- .editorconfig (1 hunks)
- .github/workflows/check_release.yml (1 hunks)
- .github/workflows/codeql.yml (1 hunks)
- .github/workflows/pythonpackage.yml (7 hunks)
- .github/workflows/release.yml (2 hunks)
- .github/workflows/upgrade_dependencies.yml (2 hunks)
- .gitignore (1 hunks)
- .pre-commit-config.yaml (1 hunks)
- CHANGES.md (1 hunks)
- Gruntfile.js (13 hunks)
- README.md (1 hunks)
- SickChill.py (9 hunks)
- contrib/runscripts/init.debian (2 hunks)
- contrib/runscripts/init.fedora (1 hunks)
- contrib/runscripts/init.systemd (1 hunks)
- contrib/runscripts/init.ubuntu (1 hunks)
- contrib/runscripts/init.upstart (2 hunks)
- debian/changelog (1 hunks)
- debian/sickchill.service (1 hunks)
- sickchill/gui/slick/css/browser.css (3 hunks)
- sickchill/gui/slick/css/dark.css (26 hunks)
- sickchill/gui/slick/css/print.css (1 hunks)
- sickchill/gui/slick/css/style.css (102 hunks)
- sickchill/gui/slick/js/ajaxEpSearch.js (2 hunks)
- sickchill/gui/slick/js/ajaxNotifications.js (1 hunks)
- sickchill/gui/slick/js/configProviders.js (3 hunks)
- sickchill/gui/slick/js/core.js (48 hunks)
- sickchill/gui/slick/js/imageSelector.js (9 hunks)
- sickchill/gui/slick/js/rootDirs.js (3 hunks)
- sickchill/gui/slick/views/404.mako (1 hunks)
- sickchill/gui/slick/views/500.mako (1 hunks)
- sickchill/gui/slick/views/IRC.mako (1 hunks)
- sickchill/gui/slick/views/addShows.mako (6 hunks)
- sickchill/gui/slick/views/addShows_addExistingShow.mako (2 hunks)
- sickchill/gui/slick/views/addShows_favoriteShows.mako (5 hunks)
- sickchill/gui/slick/views/addShows_newShow.mako (7 hunks)
- sickchill/gui/slick/views/addShows_popularShows.mako (4 hunks)
- sickchill/gui/slick/views/addShows_trendingShows.mako (2 hunks)
- sickchill/gui/slick/views/apiBuilder.mako (3 hunks)
- sickchill/gui/slick/views/config.mako (4 hunks)
- sickchill/gui/slick/views/config_anime.mako (7 hunks)
- sickchill/gui/slick/views/config_backuprestore.mako (4 hunks)
- sickchill/gui/slick/views/config_general.mako (40 hunks)
- sickchill/gui/slick/views/config_postProcessing.mako (37 hunks)
- sickchill/gui/slick/views/config_providers.mako (51 hunks)
- sickchill/gui/slick/views/config_search.mako (61 hunks)
- sickchill/gui/slick/views/config_shares.mako (2 hunks)
- sickchill/gui/slick/views/config_subtitles.mako (13 hunks)
- sickchill/gui/slick/views/displayShow.mako (9 hunks)
- sickchill/gui/slick/views/editShow.mako (25 hunks)
- sickchill/gui/slick/views/errorlogs.mako (1 hunks)
- sickchill/gui/slick/views/genericMessage.mako (1 hunks)
- sickchill/gui/slick/views/history.mako (4 hunks)
- sickchill/gui/slick/views/home.mako (3 hunks)
- sickchill/gui/slick/views/home_massAddTable.mako (3 hunks)
- sickchill/gui/slick/views/home_postprocess.mako (3 hunks)
- sickchill/gui/slick/views/inc_addShowOptions.mako (1 hunks)
Files not processed due to max files limit (183)
- sickchill/gui/slick/views/inc_blackwhitelist.mako
- sickchill/gui/slick/views/inc_home_menu.mako
- sickchill/gui/slick/views/inc_home_show_list.mako
- sickchill/gui/slick/views/inc_home_show_list_poster.mako
- sickchill/gui/slick/views/inc_qualityChooser.mako
- sickchill/gui/slick/views/layouts/config.mako
- sickchill/gui/slick/views/layouts/main.mako
- sickchill/gui/slick/views/login.mako
- sickchill/gui/slick/views/manage.mako
- sickchill/gui/slick/views/manage_backlogOverview.mako
- sickchill/gui/slick/views/manage_episodeStatuses.mako
- sickchill/gui/slick/views/manage_failedDownloads.mako
- sickchill/gui/slick/views/manage_manageSearches.mako
- sickchill/gui/slick/views/manage_massEdit.mako
- sickchill/gui/slick/views/manage_subtitleMissed.mako
- sickchill/gui/slick/views/manual_search_show_releases.mako
- sickchill/gui/slick/views/markdown.mako
- sickchill/gui/slick/views/movies/add.mako
- sickchill/gui/slick/views/movies/details.mako
- sickchill/gui/slick/views/movies/index.mako
- sickchill/gui/slick/views/movies/remove.mako
- sickchill/gui/slick/views/movies/search.mako
- sickchill/gui/slick/views/restart.mako
- sickchill/gui/slick/views/schedule.mako
- sickchill/gui/slick/views/status.mako
- sickchill/gui/slick/views/testRename.mako
- sickchill/gui/slick/views/trendingShows.mako
- sickchill/gui/slick/views/viewlogs.mako
- sickchill/helper/argument_parser.py
- sickchill/helper/common.py
- sickchill/init_helpers.py
- sickchill/logger.py
- sickchill/oldbeard/classes.py
- sickchill/oldbeard/clients/deluged.py
- sickchill/oldbeard/clients/download_station.py
- sickchill/oldbeard/clients/generic.py
- sickchill/oldbeard/clients/qbittorrent.py
- sickchill/oldbeard/clients/rtorrent.py
- sickchill/oldbeard/clients/transmission.py
- sickchill/oldbeard/common.py
- sickchill/oldbeard/config.py
- sickchill/oldbeard/dailysearcher.py
- sickchill/oldbeard/databases/main.py
- sickchill/oldbeard/filters.py
- sickchill/oldbeard/helpers.py
- sickchill/oldbeard/name_parser/parser.py
- sickchill/oldbeard/naming.py
- sickchill/oldbeard/network_timezones.py
- sickchill/oldbeard/notifiers/init.py
- sickchill/oldbeard/notifiers/boxcar2.py
- sickchill/oldbeard/notifiers/emailnotify.py
- sickchill/oldbeard/notifiers/mattermost.py
- sickchill/oldbeard/notifiers/mattermostbot.py
- sickchill/oldbeard/notifiers/nmj.py
- sickchill/oldbeard/notifiers/nmjv2.py
- sickchill/oldbeard/notifiers/plex.py
- sickchill/oldbeard/notifiers/pytivo.py
- sickchill/oldbeard/notifiers/trakt.py
- sickchill/oldbeard/notifiers/twilio_notify.py
- sickchill/oldbeard/nzbget.py
- sickchill/oldbeard/postProcessor.py
- sickchill/oldbeard/processTV.py
- sickchill/oldbeard/properFinder.py
- sickchill/oldbeard/providers/init.py
- sickchill/oldbeard/providers/abnormal.py
- sickchill/oldbeard/providers/alpharatio.py
- sickchill/oldbeard/providers/archetorrent.py
- sickchill/oldbeard/providers/bitcannon.py
- sickchill/oldbeard/providers/bjshare.py
- sickchill/oldbeard/providers/btn.py
- sickchill/oldbeard/providers/cpasbien.py
- sickchill/oldbeard/providers/danishbits.py
- sickchill/oldbeard/providers/demonoid.py
- sickchill/oldbeard/providers/elitetorrent.py
- sickchill/oldbeard/providers/eztv.py
- sickchill/oldbeard/providers/filelist.py
- sickchill/oldbeard/providers/gimmepeers.py
- sickchill/oldbeard/providers/hd4free.py
- sickchill/oldbeard/providers/hdbits.py
- sickchill/oldbeard/providers/hdspace.py
- sickchill/oldbeard/providers/hdtorrents.py
- sickchill/oldbeard/providers/hdtorrents_it.py
- sickchill/oldbeard/providers/horriblesubs.py
- sickchill/oldbeard/providers/hounddawgs.py
- sickchill/oldbeard/providers/ilcorsaronero.py
- sickchill/oldbeard/providers/immortalseed.py
- sickchill/oldbeard/providers/iptorrents.py
- sickchill/oldbeard/providers/kat.py
- sickchill/oldbeard/providers/limetorrents.py
- sickchill/oldbeard/providers/magnetdl.py
- sickchill/oldbeard/providers/morethantv.py
- sickchill/oldbeard/providers/ncore.py
- sickchill/oldbeard/providers/nebulance.py
- sickchill/oldbeard/providers/newpct.py
- sickchill/oldbeard/providers/newznab.py
- sickchill/oldbeard/providers/norbits.py
- sickchill/oldbeard/providers/nyaa.py
- sickchill/oldbeard/providers/omgwtfnzbs.py
- sickchill/oldbeard/providers/pretome.py
- sickchill/oldbeard/providers/rarbg.py
- sickchill/oldbeard/providers/rsstorrent.py
- sickchill/oldbeard/providers/scc.py
- sickchill/oldbeard/providers/scenetime.py
- sickchill/oldbeard/providers/shazbat.py
- sickchill/oldbeard/providers/skytorrents.py
- sickchill/oldbeard/providers/speedcd.py
- sickchill/oldbeard/providers/thepiratebay.py
- sickchill/oldbeard/providers/tntvillage.py
- sickchill/oldbeard/providers/tokyotoshokan.py
- sickchill/oldbeard/providers/torrent9.py
- sickchill/oldbeard/providers/torrent911.py
- sickchill/oldbeard/providers/torrent_paradise.py
- sickchill/oldbeard/providers/torrentbytes.py
- sickchill/oldbeard/providers/torrentday.py
- sickchill/oldbeard/providers/torrentleech.py
- sickchill/oldbeard/providers/torrentproject.py
- sickchill/oldbeard/providers/torrentz.py
- sickchill/oldbeard/providers/tvchaosuk.py
- sickchill/oldbeard/providers/xthor.py
- sickchill/oldbeard/providers/yggtorrent.py
- sickchill/oldbeard/sab.py
- sickchill/oldbeard/scdatetime.py
- sickchill/oldbeard/scene_numbering.py
- sickchill/oldbeard/search.py
- sickchill/oldbeard/searchBacklog.py
- sickchill/oldbeard/search_queue.py
- sickchill/oldbeard/show_queue.py
- sickchill/oldbeard/stevedore.py
- sickchill/oldbeard/subtitles.py
- sickchill/oldbeard/traktChecker.py
- sickchill/oldbeard/traktTrending.py
- sickchill/oldbeard/trakt_api/trakt.py
- sickchill/oldbeard/tvcache.py
- sickchill/providers/GenericProvider.py
- sickchill/providers/metadata/generic.py
- sickchill/providers/metadata/kodi.py
- sickchill/providers/metadata/mede8er.py
- sickchill/providers/metadata/mediabrowser.py
- sickchill/providers/metadata/ps3.py
- sickchill/providers/metadata/tivo.py
- sickchill/providers/metadata/wdtv.py
- sickchill/providers/nzb/NZBProvider.py
- sickchill/providers/torrent/TorrentProvider.py
- sickchill/settings.py
- sickchill/show/ComingEpisodes.py
- sickchill/show/History.py
- sickchill/show/Show.py
- sickchill/show/indexers/handler.py
- sickchill/show/indexers/tvdb.py
- sickchill/show_updater.py
- sickchill/start.py
- sickchill/tv.py
- sickchill/update_manager/pip.py
- sickchill/update_manager/runner.py
- sickchill/views/api/webapi.py
- sickchill/views/browser.py
- sickchill/views/changelog.py
- sickchill/views/common.py
- sickchill/views/config/anime.py
- sickchill/views/config/backup.py
- sickchill/views/config/general.py
- sickchill/views/config/notifications.py
- sickchill/views/config/post_processing.py
- sickchill/views/config/providers.py
- sickchill/views/config/search.py
- sickchill/views/config/subtitles.py
- sickchill/views/history.py
- sickchill/views/home.py
- sickchill/views/imageSelector.py
- sickchill/views/index.py
- sickchill/views/manage/add_shows.py
- sickchill/views/manage/index.py
- sickchill/views/manage/post_processing.py
- sickchill/views/manage/searches.py
- sickchill/views/server_settings.py
- tests/sickchill_tests/helper/test_common.py
- tests/sickchill_tests/show/test_show.py
- tests/test_api_v1.py
- tests/test_db.py
- tests/test_notifier.py
- tests/test_search.py
- tests/test_snatch.py
- webpack.config.js
Files not summarized due to errors (1)
- sickchill/gui/slick/css/style.css: Error: Message exceeds token limit
Files skipped from review due to trivial changes (37)
- .editorconfig
- .github/workflows/codeql.yml
- .github/workflows/release.yml
- .github/workflows/upgrade_dependencies.yml
- .gitignore
- .pre-commit-config.yaml
- CHANGES.md
- README.md
- contrib/runscripts/init.debian
- contrib/runscripts/init.ubuntu
- debian/sickchill.service
- sickchill/gui/slick/css/browser.css
- sickchill/gui/slick/css/dark.css
- sickchill/gui/slick/css/print.css
- sickchill/gui/slick/js/configProviders.js
- sickchill/gui/slick/js/core.js
- sickchill/gui/slick/views/404.mako
- sickchill/gui/slick/views/500.mako
- sickchill/gui/slick/views/IRC.mako
- sickchill/gui/slick/views/addShows.mako
- sickchill/gui/slick/views/addShows_addExistingShow.mako
- sickchill/gui/slick/views/addShows_favoriteShows.mako
- sickchill/gui/slick/views/addShows_newShow.mako
- sickchill/gui/slick/views/addShows_popularShows.mako
- sickchill/gui/slick/views/addShows_trendingShows.mako
- sickchill/gui/slick/views/apiBuilder.mako
- sickchill/gui/slick/views/config.mako
- sickchill/gui/slick/views/config_anime.mako
- sickchill/gui/slick/views/config_backuprestore.mako
- sickchill/gui/slick/views/config_postProcessing.mako
- sickchill/gui/slick/views/config_shares.mako
- sickchill/gui/slick/views/config_subtitles.mako
- sickchill/gui/slick/views/editShow.mako
- sickchill/gui/slick/views/errorlogs.mako
- sickchill/gui/slick/views/genericMessage.mako
- sickchill/gui/slick/views/home_massAddTable.mako
- sickchill/gui/slick/views/home_postprocess.mako
Additional comments (Suppressed): 145
contrib/runscripts/init.systemd (1)
- 40-43: The environment variable
SR_DATAhas been replaced withSC_DATA. Ensure that all references toSR_DATAin the codebase have been updated toSC_DATA, and that the new environment variableSC_DATAis correctly set in/etc/sickchill.conf..github/workflows/check_release.yml (4)
1-61: The workflow is well-structured and follows best practices for GitHub Actions. It's clear that it's designed to test a release string in a Python environment using the SickChill library. However, there are a few points that could be improved or verified.
21-21: The TEST_STRING environment variable is set to the input string or a default value. If the input string is not sanitized, this could potentially lead to command injection. However, as per the instructions, we can assume that the input string is pre-sanitized.
29-33: The checkout action is used with
persist-credentials: falsewhich is a good practice for public repositories to prevent accidental exposure of GitHub tokens. However, if this workflow needs to push changes back to the repository, you might need to setpersist-credentials: true.61-61: The test string is parsed with a Python command that imports the
NameParserclass from the SickChill library. If theNameParserclass or the_parse_stringmethod changes in future versions of the library, this command might fail. You should ensure that the SickChill library is pinned to a specific version that supports this command.sickchill/gui/slick/js/ajaxNotifications.js (3)
1-33: The
displayPNotifyfunction has been significantly refactored. The new implementation creates a newPNotifyobject with a more detailed configuration. The configuration now includes settings fordesktop,nonblock,maxonscreen,addclass,closer_hover,delay,hide,history,shadow,stack,styling,width, anddestroy. Ensure that these changes are compatible with the rest of the codebase and that the new settings are appropriate.37-42: A new constant
notificationDownhas been introduced. This object seems to be used as a default notification when the server is down or restarting. Ensure that thetype,title, andmessagefields are appropriate and that theoffline-noticeid is not used elsewhere.44-56: The
checkNotificationsfunction has been refactored. It now handles failures by displaying thenotificationDownmessage and clearing thenotificationTimer. On success, it sets a new timeout to callcheckNotificationsagain after 3 seconds. Ensure that this new behavior is intended and that the error handling is appropriate.sickchill/gui/slick/js/ajaxEpSearch.js (2)
- 2-7: The line
PNotify.prototype.options.maxonscreen = 5;has been removed in the new hunk. If this was intentional, ensure that it doesn't affect the functionality of the PNotify notifications in your application. If it was removed by mistake, consider adding it back.+ PNotify.prototype.options.maxonscreen = 5;
- 103-106: The
replacemethod has been replaced withreplaceAll. This change will replace all occurrences of the matched string instead of just the first one. If this was the intended behavior, then the change is fine. However, if you only wanted to replace the first occurrence, you should revert back to usingreplace..github/workflows/pythonpackage.yml (7)
56-62: The Docker metadata action version has been updated from v4 to v5. Ensure that the new version is compatible with the rest of your workflow and doesn't introduce any breaking changes.
82-88: The Python version matrix has been updated to exclude Python 3.7. Make sure that this change is intentional and that your application doesn't need to support Python 3.7.
100-106: The version of the GitHub Checkout action has been updated from v3 to v4. Verify that this new version is compatible with your workflow and doesn't introduce any breaking changes.
183-197: The versions of the Docker setup QEMU action, Docker setup Buildx action, and Docker build-push action have been updated. Ensure that these new versions are compatible with your workflow and don't introduce any breaking changes.
221-227: The version of the Docker setup QEMU action has been updated from v2 to v3. Verify that this new version is compatible with your workflow and doesn't introduce any breaking changes.
231-258: The versions of the Docker setup Buildx action, Docker login action, and Docker build-push action have been updated. Also, the condition for the Docker login action has been changed to only execute if the GitHub actor is 'miigotu' and the environment variable ACT is not set. Ensure that these changes are intentional and compatible with your workflow.
268-274: The version of the Docker build-push action has been updated from v4 to v5. Verify that this new version is compatible with your workflow and doesn't introduce any breaking changes.
contrib/runscripts/init.fedora (1)
- 24-32: The environment variable prefixes have been changed from
SR_toSC_. Ensure that all references to these variables in the codebase have been updated to reflect this change. Also, verify that any external systems or scripts that rely on these environment variables have been updated.- username=${SR_USER-sickchill} - homedir=${SR_HOME-/opt/sickchill} - datadir=${SR_DATA-/opt/sickchill} - pidfile=${SR_PIDFILE-/var/run/sickchill/sickchill.pid} - nice=${SR_NICE-} + username=${SC_USER-sickchill} + homedir=${SC_HOME-/opt/sickchill} + datadir=${SC_DATA-/opt/sickchill} + pidfile=${SC_PIDFILE-/var/run/sickchill/sickchill.pid} + nice=${SC_NICE-}sickchill/gui/slick/views/history.mako (5)
6-9: The import
from sickchill.oldbeard import scdatetime, providershas been changed fromfrom sickchill.oldbeard import sbdatetime, providers. Ensure that all references tosbdatetimein the codebase have been updated toscdatetime.25-31: The code for generating the options for the "Limit" dropdown has been refactored to use a loop instead of hardcoding each option. This is a good improvement for maintainability.
38-44: The code for generating the options for the "Layout" dropdown has been refactored to use a loop instead of hardcoding each option. This is a good improvement for maintainability.
78-81: The variable
air_datehas been updated to usescdatetimeinstead ofsbdatetime. Ensure thatscdatetimeis correctly implemented and provides the same functionality assbdatetime.145-148: The variable
air_datehas been updated to usescdatetimeinstead ofsbdatetime. Ensure thatscdatetimeis correctly implemented and provides the same functionality assbdatetime.sickchill/gui/slick/views/inc_addShowOptions.mako (6)
23-23: The checkbox for subtitles has been updated to use a new
checkedfunction. Ensure that this function correctly returns the string 'checked' when the condition is true, and an empty string when the condition is false.32-32: The label for 'Use SC Metadata' has been updated. Ensure that the new label accurately represents the functionality of the checkbox.
70-70: The checkbox for 'Season Folders' has been updated to use a new
checkedfunction. Ensure that this function correctly returns the string 'checked' when the condition is true, and an empty string when the condition is false.82-82: The checkbox for 'Anime' has been updated to use a new
checkedfunction. Ensure that this function correctly returns the string 'checked' when the condition is true, and an empty string when the condition is false.94-94: The checkbox for 'Scene Numbering' has been updated to use a new
checkedfunction. Ensure that this function correctly returns the string 'checked' when the condition is true, and an empty string when the condition is false.110-110: The 'Save as default' button no longer has the 'disabled' attribute. If this is intentional and the button should always be enabled, ignore this comment. If not, consider adding the 'disabled' attribute back.
debian/changelog (1)
- 1-10: The new hunk provides a detailed changelog for the
sickchillpackage, including the version number, status, urgency level, changes made, and the author's contact information. The changes include bug fixes, new features, and dependency updates. However, the status of the release is marked asUNRELEASEDwhich might be a mistake if the changes are already live. Also, the URL in line 7 is not closed with a parenthesis.- sickchill (2023.6.27) UNRELEASED; urgency=medium + sickchill (2023.6.27) stable; urgency=medium - Add package to homebrew repository (brew tap sickchill/sickchill until https://github.com/Homebrew/homebrew-core/pull/134927 is merged + Add package to homebrew repository (brew tap sickchill/sickchill until https://github.com/Homebrew/homebrew-core/pull/134927 is merged)contrib/runscripts/init.upstart (2)
9-11: The variable names
SR_HOME,SR_DATA, andSR_OPTShave been changed toSC_HOME,SC_DATA, andSC_OPTSrespectively. Ensure that all references to these variables in the codebase have been updated to reflect these changes.37-37: The variable names
SR_HOME,SR_DATA, andSR_OPTShave been changed toSC_HOME,SC_DATA, andSC_OPTSrespectively. This change is consistent with the changes made in lines 9-11.SickChill.py (9)
13-17: The import statements are now more organized and the
pathlibandtypingmodules have been added. This is a good practice as it improves readability and maintainability.24-30: The import statement for
NameParser, ParseResultfromsickchill.oldbeard.name_parser.parserhas been added. Ensure that this new import does not conflict with existing code and is used appropriately.45-51: The import statement for
SCWebServerfromsickchill.views.server_settingshas been updated fromSRWebServer. Ensure that all references toSRWebServerin the codebase have been updated toSCWebServer.105-129: New logic has been added to handle the case when
args.subparser_name == "test-name" and args.name. This seems to be a new feature for testing names. Ensure that this new feature is working as expected and does not introduce any breaking changes.132-144: The code for setting
settings.DATA_DIRandsettings.CONFIG_FILEhas been updated to usepathlibinstead ofos.path. This is a good practice aspathlibprovides a more intuitive way to handle filesystem paths.169-177: The call to
sickchill.start.initializenow includes additional arguments. Ensure that these new arguments are handled correctly in theinitializefunction.186-192: The
web_optionsdictionary now includes thedebugkey. Ensure that this new key is used appropriately in theSCWebServerclass.199-205: The
SRWebServerclass has been replaced with theSCWebServerclass. Ensure that all references toSRWebServerin the codebase have been updated toSCWebServer.355-364: A new static method
test_namehas been added. This seems to be a new feature for testing names. Ensure that this new feature is working as expected and does not introduce any breaking changes.sickchill/gui/slick/js/rootDirs.js (3)
121-125: The
postRootDirsfunction is a new addition. It seems to be replacing the previous$.getAJAX call with a$.postcall. Ensure that the server-side endpoint/config/general/saveRootDirsis capable of handling POST requests and that the change doesn't break any functionality. Also, consider adding error handling for the AJAX call to handle any potential failures.146-146: The
postRootDirsfunction is being called here, replacing the previousrefreshRootDirsand$.getcalls. As mentioned earlier, ensure that the server-side endpoint can handle POST requests and that the change doesn't break any functionality.166-166: The
postRootDirsfunction is being called here, replacing the previousrefreshRootDirsand$.getcalls. As mentioned earlier, ensure that the server-side endpoint can handle POST requests and that the change doesn't break any functionality.sickchill/gui/slick/js/imageSelector.js (6)
1-7: The constant
SIZES_BY_TYPEhas been renamed toimageTypeSizes. Ensure that all references to this constant in the codebase have been updated to reflect this change.38-38: The
showIDis now being fetched once and stored in a variable, instead of being fetched every time it's needed. This is a good performance improvement.96-96: The magic number
80has been replaced with a variablemargin. This is a good practice as it improves readability and maintainability of the code.150-150: The variable
$thishas been renamed toelement. Ensure that all references to this variable in the codebase have been updated to reflect this change.171-178: The constant
SIZES_BY_TYPEhas been renamed toimageTypeSizes. Ensure that all references to this constant in the codebase have been updated to reflect this change.192-192: The variable
$thishas been renamed toelement. Ensure that all references to this variable in the codebase have been updated to reflect this change.sickchill/gui/slick/views/config_general.mako (17)
9-12: The import
from sickchill.oldbeard.scdatetime import scdatetime, date_presets, time_presetshas been changed fromfrom sickchill.oldbeard.sbdatetime import sbdatetime, date_presets, time_presets. Ensure that thescdatetimemodule exists and is correctly implemented in the codebase.65-66: The
checkedfunction is used instead of the previous boolean check forsettings.LAUNCH_BROWSER. Ensure that this function is correctly implemented and returns the expected output.78-82: The
selectedfunction is used instead of the previous boolean check forsettings.DEFAULT_PAGE. Ensure that this function is correctly implemented and returns the expected output.101-102: The
checkedfunction is used instead of the previous boolean check forsettings.NO_LGMARGIN. Ensure that this function is correctly implemented and returns the expected output.111-112: The label text has been changed from "Choose hour to update shows" to "Hour to update shows". Ensure that this change is intentional and does not affect the user experience.
139-140: The
settings.ENDED_SHOWS_UPDATE_INTERVALis now directly used as the value for the input field. Ensure that this value is correctly sanitized and safe to use in this context.165-166: The
checkedfunction is used instead of the previous boolean check forsettings.TRASH_REMOVE_SHOW. Ensure that this function is correctly implemented and returns the expected output.171-172: The
checkedfunction is used instead of the previous boolean check forsettings.TRASH_ROTATE_LOGS. Ensure that this function is correctly implemented and returns the expected output.227-230: The
selectedfunction is used instead of the previous boolean check forsettings.INDEXER_DEFAULT. Ensure that this function is correctly implemented and returns the expected output.306-307: The
checkedfunction is used instead of the previous boolean check forsettings.VERSION_NOTIFY. Ensure that this function is correctly implemented and returns the expected output.316-317: The
checkedfunction is used instead of the previous boolean check forsettings.AUTO_UPDATE. Ensure that this function is correctly implemented and returns the expected output.344-345: The
checkedfunction is used instead of the previous boolean check forsettings.NOTIFY_ON_UPDATE. Ensure that this function is correctly implemented and returns the expected output.382-384: The
selectedfunction is used instead of the previous boolean check forsettings.GUI_LANG. Ensure that this function is correctly implemented and returns the expected output.403-406: The
selectedfunction is used instead of the previous boolean check forsettings.THEME_NAME. Ensure that this function is correctly implemented and returns the expected output.482-483: The
checkedfunction is used instead of the previous boolean check forsettings.CUSTOM_CSS. Ensure that this function is correctly implemented and returns the expected output.513-514: The
checkedfunction is used instead of the previous boolean check forsettings.DISPLAY_ALL_SEASONS. Ensure that this function is correctly implemented and returns the expected output.523-524: The
checkedfunction is used instead of the previous boolean check forsettings.SORT_ARTICLE. Ensure that this function is correctly implemented and returns the expected output.Gruntfile.js (12)
- 18-23: The new hunk has removed the 'exec:test', 'newrelease', and 'genchanges' tasks from the 'publish' task. If these tasks are no longer necessary, this change is fine. However, if they are still required, they should be added back to the 'publish' task.
- 'exec:test', // Run tests - 'newrelease', // Pull and merge develop to master, create and push a new release - 'genchanges' // Update CHANGES.md
33-33: The 'default' task has been added to the 'publish' task. Ensure that this task is defined and does what is expected in this context.
34-34: The 'exec:update_translations' task has been commented out. If this task is no longer necessary, this change is fine. However, if it is still required, it should be uncommented.
- // 'exec:update_translations', // Update translations + 'exec:update_translations', // Update translations
259-259: The new hunk has added a requirement for the '_get_next_version' task before running the 'bump_version' task. Ensure that this task is defined and does what is expected in this context.
276-276: The new hunk has added a requirement for the '_get_next_version' task before running the 'delete_today_tags' task. Ensure that this task is defined and does what is expected in this context.
307-307: The new hunk has added a requirement for the '_get_next_version' task before committing the updates to translations and grunt. Ensure that this task is defined and does what is expected in this context.
315-318: The new hunk has added a check for changes to the 'CHANGES.md' file and commits them with the message 'Update Changelog'. This is a good practice as it keeps the changelog up to date.
340-341: The new hunk has removed the addition of the build number and '[skip ci]' to the commit message in the 'commit_combined' task. If this information is no longer necessary, this change is fine. However, if it is still required, it should be added back to the commit message.
- if (process.env.GITHUB_RUN_ID && !isRelease) { - commitMsg += ' (build ' + process.env.GITHUB_RUN_ID + ') [skip ci]'; - }
- 348-348: The new hunk has commented out the requirement for the 'commit_changed_files' task before running the 'commit_combined' task. If this task is no longer necessary, this change is fine. However, if it is still required, it should be uncommented.
- // grunt.task.requires('commit_changed_files'); + grunt.task.requires('commit_changed_files');
371-371: The new hunk has added a requirement for the '_get_last_version' task before running the 'git_list_changes' task. Ensure that this task is defined and does what is expected in this context.
386-386: The new hunk has added requirements for the '_get_next_version' and 'exec:git_list_changes' tasks before running the 'git_tag_next_version' task. Ensure that these tasks are defined and do what is expected in this context.
448-450: The new hunk has removed the 'commit_changelog' task. If this task is no longer necessary, this change is fine. However, if it is still required, it should be added back.
- 'commit_changelog': { - cmd: function() { - const file = grunt.config('changesmd_file'); - if (!file) { - grunt.fatal('Missing file path.'); - } - let path = file.slice(0, -25); // slices 'sickchill-news/CHANGES.md' (len=25) - if (!path) { - grunt.fatal('path = "' + path + '"'); - } - let pushCmd = 'git push origin master'; - if (grunt.option('no-push')) { - grunt.log.warn('Pushing with --dry-run ...'.magenta); - pushCmd += ' --dry-run'; - } - return ['cd ' + path, 'git commit -asm "Update changelog"', 'git fetch origin', 'git rebase', - pushCmd].join(' && '); - }, - stdout: true - }sickchill/gui/slick/views/config_providers.mako (16)
1-4: No changes detected in the code. The only difference is the added whitespace in the new hunk, which doesn't affect the functionality.
18-29: The variable names
newznab_provider_listandtorrent_rss_provider_listhave been changed fromnewznabProviderListandtorrentRssProviderListrespectively. Ensure that these changes are reflected throughout the codebase to avoid any undefined variable errors.73-82: The function
sorted_provider_list()has been renamed fromsortedProviderList(). Ensure that this change is reflected throughout the codebase to avoid any undefined function errors.87-98: The function
checked()has been introduced to replace the previous method of checking if a provider is enabled. Ensure that this function is defined and works as expected.102-105: The function
sorted_provider_list()has been renamed fromsortedProviderList(). Ensure that this change is reflected throughout the codebase to avoid any undefined function errors.129-135: The function
sorted_provider_list()has been renamed fromsortedProviderList(). Ensure that this change is reflected throughout the codebase to avoid any undefined function errors.153-156: The variable
newznab_provider_listhas been renamed fromnewznabProviderList. Ensure that this change is reflected throughout the codebase to avoid any undefined variable errors.161-167: No significant changes detected. The only difference is the added whitespace in the new hunk, which doesn't affect the functionality.
173-179: No significant changes detected. The only difference is the added whitespace in the new hunk, which doesn't affect the functionality.
186-192: The function
checked()has been introduced to replace the previous method of checking if a provider is enabled. Ensure that this function is defined and works as expected.199-211: The function
hidden()has been introduced to replace the previous method of checking if a provider supports backlog. Ensure that this function is defined and works as expected.230-244: The values for the
search_moderadio buttons have been changed fromsponlyandeponlytoseasonandepisoderespectively. Ensure that these changes are reflected throughout the codebase to avoid any unexpected behavior.253-259: The function
checked()has been introduced to replace the previous method of checking if a provider is enabled. Ensure that this function is defined and works as expected.310-324: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [262-324]
The function
hidden()has been introduced to replace the previous method of checking if a provider supports backlog. Ensure that this function is defined and works as expected.
343-357: The values for the
search_moderadio buttons have been changed fromsponlyandeponlytoseasonandepisoderespectively. Ensure that these changes are reflected throughout the codebase to avoid any unexpected behavior.366-372: The function
checked()has been introduced to replace the previous method of checking if a provider is enabled. Ensure that this function is defined and works as expected.sickchill/gui/slick/views/home.mako (3)
1-14: No significant changes detected. The code seems to be reformatted with additional spaces and line breaks. Ensure that this reformatting doesn't affect the rendering of the page.
39-45: No significant changes detected. The code seems to be reformatted with additional spaces and line breaks. Ensure that this reformatting doesn't affect the rendering of the page.
61-67: No significant changes detected. The code seems to be reformatted with additional spaces and line breaks. Ensure that this reformatting doesn't affect the rendering of the page.
sickchill/gui/slick/css/style.css (14)
10-13: The order of the CSS properties has been changed. This doesn't affect the functionality but it's good to maintain a consistent order for readability and maintainability.
24-26: The color value has been changed from
whiteto#ffffff. Both represent the same color, but using the hex value is more precise and consistent.40-48: The order of the CSS properties has been changed. This doesn't affect the functionality but it's good to maintain a consistent order for readability and maintainability.
68-73: The order of the CSS properties has been changed. This doesn't affect the functionality but it's good to maintain a consistent order for readability and maintainability.
154-174: The order of the CSS properties has been changed. This doesn't affect the functionality but it's good to maintain a consistent order for readability and maintainability.
188-199: The
clear-after: top;property has been removed and replaced with a new property#sub-menu a:after. Verify if this change is intentional and if it has the desired effect.385-395: The order of the CSS properties has been changed. This doesn't affect the functionality but it's good to maintain a consistent order for readability and maintainability.
420-432: The order of the CSS properties has been changed. This doesn't affect the functionality but it's good to maintain a consistent order for readability and maintainability.
457-518: The order of the CSS properties has been changed. This doesn't affect the functionality but it's good to maintain a consistent order for readability and maintainability.
541-556: The
clear-after: top;property has been removed and replaced with a new property#sub-menu a:after. Verify if this change is intentional and if it has the desired effect.575-578: The order of the CSS properties has been changed. This doesn't affect the functionality but it's good to maintain a consistent order for readability and maintainability.
602-612: The order of the CSS properties has been changed. This doesn't affect the functionality but it's good to maintain a consistent order for readability and maintainability.
623-678: The order of the CSS properties has been changed. This doesn't affect the functionality but it's good to maintain a consistent order for readability and maintainability.
683-697: The order of the CSS properties has been changed. This doesn't affect the functionality but it's good to maintain a consistent order for readability and maintainability.
sickchill/gui/slick/views/displayShow.mako (7)
6-9: The import
from sickchill.oldbeard import subtitles, notifiers, scdatetime, network_timezones, helpershas been updated fromfrom sickchill.oldbeard import subtitles, notifiers, sbdatetime, network_timezones, helpers. Ensure thatscdatetimeis the correct module to import and that it doesn't break any functionality that relied onsbdatetime.44-44: The
selectedfunction has been updated fromselected="selected"toselected(curShow == show). Ensure that this new function correctly sets the selected option in the dropdown.150-156: The URLs in the
hrefattributes have been updated. Ensure that these new URLs are correct and lead to the expected pages.285-285: The variable
show.subtitles_sc_metadatahas been updated fromshow.subtitles_sr_metadata. Ensure that this new variable is correct and doesn't break any functionality that relied onshow.subtitles_sr_metadata.474-497: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [477-509]
The
alignattribute has been replaced with thetext-centerclass. Ensure that this change doesn't affect the layout or design of the table.
533-545: The
air_datecalculation has been updated. Ensure that this new calculation correctly calculates the air date and doesn't break any functionality that relied on the old calculation.559-565: The
hiddenfunction has been updated fromhidden, ''tohidden(epResult['location'] and settings.USE_KODI and settings.KODI_HOST). Ensure that this new function correctly hides the element when necessary.sickchill/gui/slick/views/config_search.mako (26)
1-4: No changes detected in the code. The only difference is the additional whitespace at the beginning of the lines, which doesn't affect the functionality.
36-42: The way of checking the checkbox has been changed from a ternary operation to a function call
checked(). Ensure that this function is correctly implemented and returns the expected values.47-55: The way of checking the checkbox and hiding the content has been changed from a ternary operation to a function call
checked()andhidden(). Ensure that these functions are correctly implemented and return the expected values.62-67: The way of selecting the option has been changed from a ternary operation to a function call
selected(). Ensure that this function is correctly implemented and returns the expected values.77-83: No changes detected in the code. The only difference is the additional whitespace at the end of the lines, which doesn't affect the functionality.
103-109: No changes detected in the code. The only difference is the additional whitespace at the end of the lines, which doesn't affect the functionality.
123-129: No changes detected in the code. The only difference is the additional whitespace at the end of the lines, which doesn't affect the functionality.
143-149: No changes detected in the code. The only difference is the additional whitespace at the end of the lines, which doesn't affect the functionality.
162-168: No changes detected in the code. The only difference is the additional whitespace at the end of the lines, which doesn't affect the functionality.
180-186: No changes detected in the code. The only difference is the additional whitespace at the end of the lines, which doesn't affect the functionality.
199-205: No changes detected in the code. The only difference is the additional whitespace at the end of the lines, which doesn't affect the functionality.
218-224: No changes detected in the code. The only difference is the additional whitespace at the end of the lines, which doesn't affect the functionality.
237-243: No changes detected in the code. The only difference is the additional whitespace at the end of the lines, which doesn't affect the functionality.
256-262: The way of checking the checkbox has been changed from a ternary operation to a function call
checked(). Ensure that this function is correctly implemented and returns the expected values.268-274: The way of checking the checkbox has been changed from a ternary operation to a function call
checked(). Ensure that this function is correctly implemented and returns the expected values.280-283: The way of hiding the content has been changed from a ternary operation to a function call
hidden(). Ensure that this function is correctly implemented and returns the expected values.290-296: The way of checking the checkbox has been changed from a ternary operation to a function call
checked(). Ensure that this function is correctly implemented and returns the expected values.314-320: No changes detected in the code. The only difference is the additional whitespace at the end of the lines, which doesn't affect the functionality.
415-421: The way of selecting the option has been changed from a ternary operation to a function call
selected(). Ensure that this function is correctly implemented and returns the expected values.430-436: No changes detected in the code. The only difference is the additional whitespace at the end of the lines, which doesn't affect the functionality.
450-456: No changes detected in the code. The only difference is the additional whitespace at the end of the lines, which doesn't affect the functionality.
512-518: No changes detected in the code. The only difference is the additional whitespace at the end of the lines, which doesn't affect the functionality.
532-538: No changes detected in the code. The only difference is the additional whitespace at the end of the lines, which doesn't affect the functionality.
552-558: No changes detected in the code. The only difference is the additional whitespace at the end of the lines, which doesn't affect the functionality.
382-390: The way of checking the checkbox and hiding the content has been changed from a ternary operation to a function call
checked()andhidden(). Ensure that these functions are correctly implemented and return the expected values.397-403: No changes detected in the code. The only difference is the additional whitespace at the end of the lines, which doesn't affect the functionality.
|
@coderabbitai help |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 11
Files selected for processing (8)
- .github/workflows/check_release.yml (1 hunks)
- .github/workflows/pythonpackage.yml (1 hunks)
- .github/workflows/release.yml (1 hunks)
- .github/workflows/upgrade_dependencies.yml (1 hunks)
- sickchill/gui/slick/js/ajaxNotifications.js (1 hunks)
- sickchill/gui/slick/js/imageSelector.js (9 hunks)
- sickchill/gui/slick/views/history.mako (4 hunks)
- sickchill/gui/slick/views/inc_addShowOptions.mako (1 hunks)
Files skipped from review due to trivial changes (2)
- .github/workflows/check_release.yml
- .github/workflows/upgrade_dependencies.yml
Additional comments (Suppressed): 35
.github/workflows/release.yml (3)
42-45: The
Bump versionstep has been modified and the working directory is not set anymore. Also, the command to print the version usingscmaintoolsscript is removed. If this is intentional, ensure that the version bumping is handled correctly elsewhere in the codebase.32-32: The
actions/checkoutaction version has been updated fromv3tov4. Ensure that this change is compatible with the rest of the workflow and doesn't introduce any breaking changes.37-37: The
actions/setup-pythonaction version remains atv4, but thecheck-latest: trueoption is removed in the new hunk. This option ensures that the latest version of Python is always used. If this is intentional, ensure that it doesn't affect the workflow.sickchill/gui/slick/js/ajaxNotifications.js (4)
1-33: The
displayPNotifyfunction has been updated to create a new PNotify notification with more options. The options are now set for each notification instead of modifying thePNotify.prototype.optionsobject. This change provides more flexibility as it allows different options for different notifications. However, ensure that this change does not affect other parts of the code that rely on the old behavior.37-42: A new
notificationDownobject has been introduced. It defines the type, title, and message for a notification that is displayed when an error occurs during the AJAX request in thecheckNotificationsfunction. This is a good practice as it separates the error handling logic from the function.44-57: The
checkNotificationsfunction has been updated to handle failures of the AJAX request. If the request fails, it displays a notification using thenotificationDownobject and clears the timer. If the request is successful, it sets a new timer to call the function again after 3 seconds. This is a good improvement as it provides feedback to the user when an error occurs and prevents unnecessary requests when the server is down.59-59: The document ready event handler has been updated to use an arrow function. This is a minor change and does not affect the functionality.
sickchill/gui/slick/js/imageSelector.js (7)
35-41: The
showIDvariable is now defined once at the top of the script, instead of being fetched each timefetchImages()is called. This is a good performance improvement, as it reduces unnecessary DOM queries.47-53: The
showparameter in the$.getJSON()call has been updated to use theshowIDvariable instead of fetching the value from the DOM each time. This is a good performance improvement, as it reduces unnecessary DOM queries.70-81: The
altattribute of the image element has been updated to include theshowIDandimageType. This is a good improvement for accessibility, as it provides more context for screen readers.102-111: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [92-111]
The
marginvariable has been introduced to replace the hardcoded value of80. This is a good improvement for maintainability, as it makes it easier to change the margin value in the future.
146-152: The variable name
$thishas been changed toelement. This is a good improvement for readability, aselementis more descriptive than$this.170-179: The
SIZES_BY_TYPEobject has been renamed toimageTypeSizes. This is a good improvement for readability, asimageTypeSizesis more descriptive.188-198: The variable name
$thishas been changed toelement. This is a good improvement for readability, aselementis more descriptive than$this.sickchill/gui/slick/views/history.mako (8)
6-9: The import
from sickchill.oldbeard import scdatetime, providershas been updated fromfrom sickchill.oldbeard import sbdatetime, providers. Ensure that thescdatetimemodule is correctly implemented and used throughout the codebase.25-31: The code for generating the options for the "Limit" dropdown has been refactored to use a loop, which is a more efficient and maintainable approach. This change is approved.
38-44: The code for generating the options for the "Layout" dropdown has been refactored to use a loop, which is a more efficient and maintainable approach. This change is approved.
78-81: The
air_dateandisoDatevariables have been updated to use thescdatetimemodule instead of thesbdatetimemodule. Ensure that thescdatetimemodule is correctly implemented and used throughout the codebase.101-106: The code for handling the provider image has been updated to handle a missing provider. This is a good addition for error handling.
145-148: The
air_dateandisoDatevariables have been updated to use thescdatetimemodule instead of thesbdatetimemodule. Ensure that thescdatetimemodule is correctly implemented and used throughout the codebase.159-171: The code for handling the provider image has been updated to handle a missing provider. This is a good addition for error handling.
187-195: The code for handling the subtitles image has been updated to handle a missing provider. This is a good addition for error handling.
.github/workflows/pythonpackage.yml (13)
56-77: The Docker meta step has been updated to use version 5 of the
docker/metadata-action. Ensure that the new version is compatible with the rest of the workflow and that the changes in thetagsandlabelsfields are correct.81-90: The Python versions for testing have been updated to include 3.8, 3.9, 3.10, and 3.11. The operating systems for testing now include only Ubuntu. Ensure that these changes are intentional and that they do not exclude any necessary test cases.
97-98: The Checkout step has been updated to use version 4 of the
actions/checkoutaction. Ensure that the new version is compatible with the rest of the workflow.107-113: The Install Poetry step has been updated to use version 1.3.2 of the
snok/install-poetryaction and version 1.2.0 of Poetry. Ensure that these new versions are compatible with the rest of the workflow.115-118: The Set up Python step has been updated to use version 4 of the
actions/setup-pythonaction. Ensure that the new version is compatible with the rest of the workflow.121-124: The Set up Node step has been updated to use version 3 of the
actions/setup-nodeaction. Ensure that the new version is compatible with the rest of the workflow.163-167: The Set up Docker Buildx step has been updated to use version 3 of the
docker/setup-buildx-actionaction. Ensure that the new version is compatible with the rest of the workflow.169-182: The Test Docker Build step has been updated to use version 5 of the
docker/build-push-actionaction. Ensure that the new version is compatible with the rest of the workflow.197-207: The Set up Docker Buildx step in the Deploy job has been updated to use version 3 of the
docker/setup-buildx-actionaction. Ensure that the new version is compatible with the rest of the workflow.210-213: The Login to DockerHub step has been updated to use version 3 of the
docker/login-actionaction. Ensure that the new version is compatible with the rest of the workflow.215-220: The Login to GitHub Container Registry step has been updated to use version 3 of the
docker/login-actionaction. Ensure that the new version is compatible with the rest of the workflow.222-235: The Build and Push step has been updated to use version 5 of the
docker/build-push-actionaction. Ensure that the new version is compatible with the rest of the workflow.237-249: The Snag wheels step has been updated to use version 5 of the
docker/build-push-actionaction. Ensure that the new version is compatible with the rest of the workflow.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files selected for processing (1)
- sickchill/gui/slick/js/imageSelector.js (9 hunks)
Additional comments (Suppressed): 8
sickchill/gui/slick/js/imageSelector.js (8)
1-5: The constant
SIZES_BY_TYPEhas been renamed toimageTypeSizes. Ensure that all references to this constant in the codebase have been updated to reflect this change.36-36: The
showIDvariable has been moved to a higher scope. This change is fine as long as theshowIDvalue does not need to be refreshed every timefetchImages()is called.48-48: The
showparameter in the AJAX request has been updated to use theshowIDvariable instead of fetching the value from the#showIDelement each time. This is a good change for performance, as long as theshowIDvalue does not change after the initial assignment.71-71: The
altattribute has been added to the image element. This is a good practice for accessibility and SEO.93-93: The
marginvariable has been introduced to replace the hardcoded value80. This is a good practice for maintainability.147-147: The variable
$thishas been renamed toelement. This is a good change for readability.171-176: The
SIZES_BY_TYPEconstant has been replaced withimageTypeSizes. Ensure that all references to this constant in the codebase have been updated to reflect this change.189-192: The
clickevent handler has been updated to useelementinstead of$(this). This is a good change for readability.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Files selected for processing (4)
- .github/workflows/pythonpackage.yml (1 hunks)
- sickchill/gui/slick/js/core.js (49 hunks)
- sickchill/gui/slick/js/imageSelector.js (9 hunks)
- sickchill/gui/slick/views/inc_addShowOptions.mako (1 hunks)
Files skipped from review due to trivial changes (1)
- .github/workflows/pythonpackage.yml
Additional comments (Suppressed): 20
sickchill/gui/slick/views/inc_addShowOptions.mako (2)
30-30: The checkbox for "Use SC Metadata" is not preserving its state based on the default settings. This could lead to unexpected behavior for the user. However, as per the previous comment chain, the default state for this checkbox is always unchecked. Therefore, the current implementation is correct.
70-70: The checkbox for "Season Folders" is preserving its state based on the default settings using the
checked()function. As per the previous comment chain,checked()is a function that correctly handles the checkbox state based on a boolean input. Therefore, the current implementation is correct.sickchill/gui/slick/js/imageSelector.js (8)
1-5: The constant
SIZES_BY_TYPEhas been renamed toimageTypeSizes. Ensure that all references to this constant in the codebase have been updated to reflect this change.33-38: The
showIDvariable has been moved from thefetchImagesfunction to the outer scope. This change is fine as long as theshowIDvalue does not need to be updated every timefetchImagesis called.45-58: The
showparameter in the AJAX request has been updated from$('#showID').attr('value')toshowID. This change is fine as long as theshowIDvalue does not need to be updated every timefetchImagesis called.70-79: The
altattribute has been added to the image element created in thecreateImagefunction. This is a good practice for accessibility and SEO.101-110: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [91-110]
The
marginandminimumWidthvariables have been moved from thedialogfunction to the outer scope. This change is fine as long as these values do not need to be updated every time$.fn.nImageSelectoris called.
145-151: The
$thisvariable has been renamed toelement. Ensure that all references to this variable in the codebase have been updated to reflect this change.169-178: The
SIZES_BY_TYPEconstant has been replaced withimageTypeSizesin theloadFunction. Ensure that all references to this constant in the codebase have been updated to reflect this change.187-196: The
$thisvariable has been replaced withelementin theclickevent handler. Ensure that all references to this variable in the codebase have been updated to reflect this change.sickchill/gui/slick/js/core.js (10)
14-27: The function
configSuccessnow has an optional parameterreloadwhich defaults totrue. Ifreloadistrue, the page will be reloaded after the function is executed. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.32-37: The function
metaToBoolnow returnsmetawhenmetaisundefined. This is a change from the previous behavior where it would returnundefineddirectly. Ensure this change does not affect the logic where this function is used.123-126: The text in the shutdown confirmation dialog has been changed from 'Are you sure you want to shutdown SickChill?' to 'Are you sure you want to shut down SickChill?'. Ensure this change aligns with the intended user experience.
235-241: The event listener for
.bulkCheckhas been changed from aclickevent to achangeevent. This change makes sense if you want the event to be triggered whenever the checkbox state changes, not just when it's clicked. Ensure this change aligns with the intended functionality.310-314: The
clickevent handler for.viewIfhas been changed to usethis.checkedinstead of$(this).is(':checked'). Both are equivalent, butthis.checkedis more performant as it doesn't involve a jQuery function call.344-351: The loading message has been changed from 'Saving...' to ' Saving...'. Ensure this change aligns with the intended user experience.
365-373: The error handling logic for the
generate_new_apikeyclick event has been changed. Previously, if there was an error, the function would return early. Now, it continues to execute even if there's an error. Ensure this change does not introduce unintended behavior.689-692: New functionality has been added to handle a click event on
#testMattermostBot. Ensure this new functionality has been thoroughly tested.1506-1539: The logic for handling the
naming_anime_patterninput has been changed. Previously, the input's background color was set to#FFFFFFwhen the pattern was valid. Now, the input's background color is set to#FFFFFFregardless of whether the pattern is valid or not. Ensure this change does not affect the user experience.2104-2110: The checkbox in the service list is now checked by default. Ensure this change aligns with the intended functionality.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files selected for processing (1)
- sickchill/gui/slick/js/imageSelector.js (8 hunks)
Additional comments (Suppressed): 7
sickchill/gui/slick/js/imageSelector.js (7)
1-5: The constant
SIZES_BY_TYPEhas been renamed toimageTypeSizes. Ensure that all references to this constant in the codebase have been updated to reflect this change.51-57: The condition
entry.hasOwnProperty('thumb') && entry.hasOwnProperty('image')has been added. This is a good practice as it ensures that the properties 'thumb' and 'image' exist in the 'entry' object before trying to access them, thus preventing potential errors.70-75: The
altattribute has been added to theimgelement. This is a good practice for accessibility and SEO. However, ensure that the$('#showID').attr('value')andimageTypevalues are always defined and sanitized to prevent potential XSS attacks.90-94: The variables
marginandminimumWidthhave been introduced to replace the hardcoded values80and650respectively. This is a good practice as it improves readability and maintainability of the code.144-150: The variable
$thishas been renamed toelement. Ensure that all references to this variable in the codebase have been updated to reflect this change.168-176: The constant
SIZES_BY_TYPEhas been renamed toimageTypeSizes. Ensure that all references to this constant in the codebase have been updated to reflect this change.189-195: The variable
$thishas been renamed toelement. Ensure that all references to this variable in the codebase have been updated to reflect this change.
Signed-off-by: miigotu <[email protected]>
Signed-off-by: miigotu <[email protected]>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Files selected for processing (9)
- .github/workflows/check_release.yml (1 hunks)
- .github/workflows/pythonpackage.yml (1 hunks)
- .github/workflows/release.yml (1 hunks)
- .github/workflows/upgrade_dependencies.yml (1 hunks)
- sickchill/gui/slick/js/ajaxNotifications.js (1 hunks)
- sickchill/gui/slick/js/core.js (49 hunks)
- sickchill/gui/slick/js/imageSelector.js (8 hunks)
- sickchill/gui/slick/views/history.mako (4 hunks)
- sickchill/gui/slick/views/inc_addShowOptions.mako (1 hunks)
Files skipped from review due to trivial changes (2)
- .github/workflows/pythonpackage.yml
- sickchill/gui/slick/views/inc_addShowOptions.mako
Additional comments (Suppressed): 42
.github/workflows/check_release.yml (4)
1-52: The workflow is well-structured and follows best practices for GitHub Actions. It's good to see the use of concurrency to avoid redundant runs and the use of environment variables to pass data between steps. However, please ensure that the new command-line argument
--test-nameis implemented and tested in thesickchillapplication. Also, verify that the default test string "Show.Name.S01E01.HDTV.h265-SICKCHILL" is appropriate for most cases.28-28: The
actions/checkout@v3action is being used, but at the time of my knowledge cutoff in September 2021, the latest version wasv2. Please verify thatv3is now available and stable.40-40: The
actions/setup-python@v4action is being used, but at the time of my knowledge cutoff in September 2021, the latest version wasv2. Please verify thatv4is now available and stable.42-42: Python 3.11 is being used. As of my knowledge cutoff in September 2021, the latest stable version was Python 3.9. Please verify that Python 3.11 is now stable and compatible with the
sickchillapplication and its dependencies..github/workflows/upgrade_dependencies.yml (5)
13-16: The
strategy.matrix.oshas been changed from a fixed versionubuntu-22.04toubuntu-latest. This could potentially introduce breaking changes if future versions of Ubuntu are not compatible with the workflow. Please verify that the workflow is compatible with the latest Ubuntu version.17-17: The
runs-onvalue has been changed to use the matrix OS value. This is a good change as it allows for easier updates to the OS version in the future.21-21: The
actions/checkoutaction version has been updated fromv3tov4. Please verify that the workflow is compatible with the new version of the action.34-37: The Python version has been updated from
3.9to3.11. Please verify that the workflow and the application are compatible with the new Python version.49-52: The
stefanzweifel/git-auto-commit-actionaction version has been updated fromv4tov5. Please verify that the workflow is compatible with the new version of the action..github/workflows/release.yml (4)
21-21: The condition
github.actor == 'miigotu'restricts the workflow to run only when the actor is 'miigotu'. Ensure this is the intended behavior and that it doesn't limit necessary workflow runs.32-32: The
actions/checkoutaction version has been updated from v3 to v4. Ensure that this version change doesn't introduce any breaking changes.34-35: The
submodules: "recursive"option has been removed from theactions/checkoutaction. If the repository has submodules, they won't be checked out. Verify if this is the intended behavior.47-49: The
Publishstep remains unchanged, which is good as it ensures the package is still published to PyPI using the stored secrets.sickchill/gui/slick/js/imageSelector.js (8)
1-5: The constant
SIZES_BY_TYPEhas been renamed toimageTypeSizes. Ensure that all references to this constant in the codebase have been updated to reflect this change.51-57: The
hasOwnPropertymethod has been added to check if theentryobject hasthumbandimageproperties. This is a good practice to avoid potential errors when accessing properties of an object.70-78: The
altattribute has been added to theimgelement, which improves accessibility. However, the concatenation of thealtattribute value could be prone to XSS attacks if the values of$('#showID').attr('value')andimageTypeare not properly sanitized. Since the instructions specify that arguments to internal function calls are pre-sanitized, this should not be an issue, but it's worth noting.90-96: The variables
marginandminimumWidthhave been added to improve readability and maintainability of the code. This is a good practice as it makes the code easier to understand and modify in the future.100-109: The variables
marginandminimumWidthare now being used in the dialog configuration. This improves readability and maintainability of the code.144-150: The variable
$thishas been renamed toelement. Ensure that all references to this variable in the codebase have been updated to reflect this change.168-176: The constant
SIZES_BY_TYPEhas been renamed toimageTypeSizes. Ensure that all references to this constant in the codebase have been updated to reflect this change.186-195: The variable
$thishas been renamed toelement. Ensure that all references to this variable in the codebase have been updated to reflect this change.sickchill/gui/slick/views/history.mako (8)
6-9: The import
from sickchill.oldbeard import scdatetime, providershas been updated fromfrom sickchill.oldbeard import sbdatetime, providers. Ensure that thescdatetimemodule is correctly implemented and used throughout the codebase.25-31: The code for generating the options for the "Limit" dropdown has been refactored to use a loop, which is a more efficient and maintainable approach. This is a good improvement.
38-44: The code for generating the options for the "Layout" dropdown has been refactored to use a loop, which is a more efficient and maintainable approach. This is a good improvement.
79-81: The
scdatetimemodule is used here instead of the oldsbdatetimemodule. Ensure that thescdatetimemodule is correctly implemented and used throughout the codebase.145-148: The
scdatetimemodule is used here instead of the oldsbdatetimemodule. Ensure that thescdatetimemodule is correctly implemented and used throughout the codebase.159-171: The code for generating the provider icons has been refactored to use a loop, which is a more efficient and maintainable approach. This is a good improvement.
173-183: The code for generating the provider names has been refactored to use a loop, which is a more efficient and maintainable approach. This is a good improvement.
185-196: The code for generating the subtitles icons has been refactored to use a loop, which is a more efficient and maintainable approach. This is a good improvement.
sickchill/gui/slick/js/ajaxNotifications.js (4)
1-35: The
displayPNotifyfunction has been updated to include more options for the PNotify notification. The options are now defined within the function call instead of modifying thePNotify.prototype.optionsobject. This is a good practice as it avoids modifying the prototype and potentially affecting other parts of the code that use PNotify. The function now also callsPNotify.desktop.permission()before creating a notification, which is necessary for desktop notifications. Thedestroyoption has been added, which will remove the notification from the DOM after it disappears. This can help to improve performance if there are many notifications. The styling has been changed from 'bootstrap3' to 'fontawesome', which will affect the appearance of the notifications. Ensure that this change is intentional and that the FontAwesome library is included in the project.37-42: A new constant
notificationDownhas been defined, which contains the details for an 'offline' notification. This is a good practice as it avoids repeating these details in multiple places.44-57: The
checkNotificationsfunction has been updated to handle failures in the AJAX request. If the request fails, it displays the 'offline' notification and stops checking for new notifications. This is a good practice as it provides feedback to the user when there is a problem. However, consider adding a way to resume checking for notifications once the server is back online.function checkNotifications() { $.getJSON(scRoot + '/ui/get_messages', data => { $.each(data, (name, data) => { displayPNotify(data.type, data.title, data.message, data.hash); }); }) .fail(() => { displayPNotify(notificationDown.type, notificationDown.title, notificationDown.message, 'offline-notice'); clearTimeout(notificationTimer); notificationTimer = setTimeout(checkNotifications, 30000); // Check again after 30 seconds }) .done(() => { notificationTimer = setTimeout(checkNotifications, 3000); }); }
- 59-59: The document ready event handler has been removed from this hunk. Ensure that it is included elsewhere in the code, as it is necessary to start checking for notifications when the document is ready.
sickchill/gui/slick/js/core.js (9)
- 12-21: The function
configSuccesshas been modified to include an optionalreloadparameter. Ifreloadistrue, the page will be reloaded. Ensure that all calls toconfigSuccessthroughout the codebase have been updated to match the new signature.- function configSuccess() { + function configSuccess(reload = true) {
- 14-21: The function
configSuccesshas been modified to include an optionalreloadparameter. Ifreloadistrue, the page will be reloaded. Ensure that all calls toconfigSuccessthroughout the codebase have been updated to match the new signature.- function configSuccess() { + function configSuccess(reload = true) {
- 123-126: The text in the shutdown confirmation dialog has been slightly modified. Ensure that this change aligns with the intended user experience.
- text: 'Are you sure you want to shutdown SickChill?', + text: 'Are you sure you want to shut down SickChill?',
- 196-202: The jQuery
propmethod is used instead ofattrto get theariaExpandedandhrefproperties of the element. This is a more appropriate method to use for properties that have true or false values.- if ($this.attr('aria-expanded') === 'true') { - window.location.href = $this.attr('href'); + if (element.prop('ariaExpanded') === 'true') { + window.location.href = element.prop('href');
- 235-241: The event handler for the
.bulkCheckchange event has been modified. The new code directly accesses thecheckedproperty of the event target, which is more efficient than the previous jQuery-based approach.- const element = $(event.currentTarget); - const whichBulkCheck = element.attr('id'); - $('.' + whichBulkCheck + ':visible').each(function () { - $(this).prop('checked', element.is(':checked')); - }); + const checkbox = event.currentTarget; + const childrenClass = '.' + checkbox.id + ':visible'; + $(childrenClass).each(function () { + this.checked = checkbox.checked; + });
- 1272-1276: The event handler for the
#use_plex_serverclick event has been modified. The new code directly accesses thecheckedproperty of the event target, which is more efficient than the previous jQuery-based approach.- if ($(this).is(':checked')) { + if (this.checked) {
- 1287-1290: The
typewatchfunction has been slightly modified. Thetimervariable is now initialized withlet timer;instead oflet timer = 0;. This change should not affect the functionality of the code, asclearTimeoutcan handleundefinedvalues.- let timer = 0; + let timer;
- 1506-1539: The
namingAnimePatternInputjQuery object has been replaced with direct DOM access. This is a more efficient approach, but ensure that all instances ofnamingAnimePatternInputin the codebase have been updated to reflect this change.- const $namingAnimePatternInput = $('#naming_anime_pattern'); + const namingAnimePatternInput = $('#naming_anime_pattern');
- 1718-1735: The
configArrayis now built using multiplepushcalls instead of a single call with multiple arguments. This change improves readability and maintainability.- configArray.push(showMetadata ? '1' : '0', episodeMetadata ? '1' : '0', fanart ? '1' : '0', poster ? '1' : '0', banner ? '1' : '0', episodeThumbnails ? '1' : '0', seasonPosters ? '1' : '0', seasonBanners ? '1' : '0', seasonAllPoster ? '1' : '0', seasonAllBanner ? '1' : '0'); + configArray.push( + showMetadata ? '1' : '0', + episodeMetadata ? '1' : '0', + fanart ? '1' : '0', + poster ? '1' : '0', + banner ? '1' : '0', + episodeThumbnails ? '1' : '0', + seasonPosters ? '1' : '0', + seasonBanners ? '1' : '0', + seasonAllPoster ? '1' : '0', + seasonAllBanner ? '1' : '0', + );
Signed-off-by: miigotu <[email protected]>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Files selected for processing (4)
- .github/workflows/check_release.yml (1 hunks)
- .github/workflows/pythonpackage.yml (1 hunks)
- .github/workflows/release.yml (1 hunks)
- .github/workflows/upgrade_dependencies.yml (3 hunks)
Files skipped from review due to trivial changes (1)
- .github/workflows/upgrade_dependencies.yml
Additional comments (Suppressed): 21
.github/workflows/check_release.yml (1)
- 1-52: The workflow is well-structured and follows best practices for GitHub Actions. It checks out the code, sets up the environment, installs dependencies, and runs the test. However, please ensure that the
sickchill --test-namecommand is correctly implemented and handles theTEST_STRINGenvironment variable properly. Also, verify that theTEST_STRINGdefault value is appropriate for most test cases..github/workflows/release.yml (4)
33-35: The
actions/checkoutaction has been updated from version 3 to version 4. Ensure that this change does not introduce any breaking changes or unexpected behavior.37-41: The
actions/setup-pythonaction has been updated from version 3 to version 4. Ensure that this change does not introduce any breaking changes or unexpected behavior.43-47: The
Bump versionstep has been modified. The previous version was running a script from thesickchill/contrib/scmaintoolsdirectory, but the new version does not specify a working directory and does not run the script. If the script is still necessary, consider adding it back.- run: | - poetry install - # Set release version to todays date - # debchange with gbd-change with new version + working-directory: sickchill/contrib/scmaintools + run: | + poetry install + python scmaintools -n
- 33-36: The
submodules: "recursive"option is missing in the new hunk. This option is used to recursively fetch all submodules for the repository. If the project relies on submodules, this could lead to missing dependencies or broken code. However, as per the previous comment chain, it was clarified that submodules are no longer in use, so this change is acceptable..github/workflows/pythonpackage.yml (16)
56-77: The Docker metadata action version has been updated from v4 to v5. Ensure that all the parameters and their values are compatible with the new version.
82-90: The Python versions for testing have been updated and the OS matrix has been expanded to include Windows and MacOS. Ensure that the project is compatible with these environments and that all necessary dependencies are available.
97-98: The version of the checkout action has been updated from v3 to v4. Ensure that this change does not affect the workflow.
103-105: A new step has been added to lint shell scripts. This is a good practice to catch potential issues in shell scripts.
107-114: The poetry installation and Python setup steps have been updated. Ensure that these changes are compatible with the project's requirements.
115-119: The version of the setup-python action has been updated from v4 to v5. Ensure that this change does not affect the workflow.
123-128: Node setup and yarn installation steps have been added. Ensure that these steps are necessary for the project and that all dependencies are available.
144-147: Linting and testing steps for JavaScript have been added. This is a good practice to catch potential issues in JavaScript code.
152-152: The condition for running the coverage step has been updated to exclude dependabot. This is a good practice to avoid unnecessary runs of the coverage step.
164-168: The versions of the setup-qemu-action and setup-buildx-action have been updated from v2 to v3. Ensure that these changes do not affect the workflow.
170-182: The version of the build-push-action has been updated from v4 to v5. Ensure that all the parameters and their values are compatible with the new version.
188-190: The checks for the Docker container have been updated. Ensure that these checks are accurate and sufficient for the project's requirements.
211-221: The versions of the login-action have been updated from v2 to v3. Ensure that these changes do not affect the workflow.
224-236: The version of the build-push-action has been updated from v4 to v5. Ensure that all the parameters and their values are compatible with the new version.
239-250: A new step has been added to build and push Docker images. This is a good practice for automating the deployment process.
255-269: The paths for uploading wheels have been updated. Ensure that these paths are correct and that the necessary permissions are in place.
| @@ -18,40 +18,33 @@ concurrency: | |||
| jobs: | |||
| Release: | |||
| runs-on: ubuntu-22.04 | |||
| if: github.event.workflow_run.conclusion == 'success' && github.actor == 'miigotu' | |||
| if: github.event.workflow_run.conclusion == "success" && github.actor == "miigotu" | |||
There was a problem hiding this comment.
The indentation of the if condition is incorrect. It should be aligned with runs-on.
- if: github.event.workflow_run.conclusion == "success" && github.actor == "miigotu"
+ if: github.event.workflow_run.conclusion == "success" && github.actor == "miigotu"
Summary by CodeRabbit
test_nameto theSickChillclass for testing names and parsing results.bdecodeandtorrentleech.