Skip to content

Add test in case of the settings isn't set due to module upgrade#59

Merged
valadas merged 1 commit into
DNNCommunity:developfrom
stetard:Download-Notification-Error-After-Upgrade
Dec 8, 2023
Merged

Add test in case of the settings isn't set due to module upgrade#59
valadas merged 1 commit into
DNNCommunity:developfrom
stetard:Download-Notification-Error-After-Upgrade

Conversation

@stetard

@stetard stetard commented Nov 7, 2023

Copy link
Copy Markdown
Contributor

Closes #57

Just add a test in case of the setting doesn't exist in the database.
It could occurs on old Repository modules after an upgrade.

PR Template Checklist

  • Fixes Bug
  • Feature solution
  • Other

Please mark which issue is solved

Close #57

@stetard stetard closed this Nov 7, 2023
@stetard stetard reopened this Nov 8, 2023
@valadas valadas changed the title Add test in case of the settings isn't set due to module upgrade (clo… Add test in case of the settings isn't set due to module upgrade Dec 8, 2023
@valadas

valadas commented Dec 8, 2023

Copy link
Copy Markdown
Member

@stetard I read the whole PR, I see a lot of refactoring but not seeing any meaningful change, did I miss something, can you point me to the line number where the fix starts?

@valadas

valadas commented Dec 8, 2023

Copy link
Copy Markdown
Member

Oh, I reread and found it, merging.
It would make it easier/quicker to review if major style refactorings and the bugfix are done in separate commits.

@valadas
valadas merged commit 2f0fe55 into DNNCommunity:develop Dec 8, 2023
@stetard

stetard commented Dec 8, 2023

Copy link
Copy Markdown
Contributor Author

OK Daniel
Thanks for your feedback.

@stetard
stetard deleted the Download-Notification-Error-After-Upgrade branch December 9, 2023 02:04
@stetard
stetard restored the Download-Notification-Error-After-Upgrade branch December 9, 2023 02:04
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.

Error on download notification after module upgrade

2 participants