531: Optimizing email and issuing task. #632
Conversation
|
Hi @mdjnelson, this is the PR, Claud's changes are pretty much the same as some of the Mohamed code. We are good with this I guess. Let me know what you think. |
|
Hi @ojnadjarm, thanks for this. Have you seen the comments I left on Mohamed's PR at #610? There are a few things I would like addressed. Also please prepend #531 to your commit message so people know that it is related to a current issue. |
ojnadjarm
changed the title
|
@mdjnelson Yes, I think I addresed the comments you did on the #610 PR. I guess is about the settings mostly? I do found both valuable, the invisible courses, ppl do weird stuff and I do think a even with a course/category hidden the user may still want to send the certificates, so give the option to do so I don't think is harmful, also the expiration I think is necesary, we needed a way to skip really old certificates like a course with 2 years old endate or a certificate that hasn't issue in a while 1 or 2 years, the user will decide that, yet we are processing them on the loop and overcharging the task. Those are the main issues I could think of, there are another issues you are refering to? |
|
Hi Mark, ojnadjarm I apologize for the delay in responding as I was on an extended leave for nearly two months and did not review emails during that time. Regarding the hidden categories and courses, I configured them to allow administrators to tailor settings according to their organization's needs. Different organizations have varied operational methods. For instance, some universities have between 200K to 500K enrollments per term and retain courses on Moodle for 3 to 10 years. These older courses are usually hidden and are rarely accessed for certificate issuance. Therefore, it might not be efficient to include these in certificate checks for the entire 10 years. Allowing administrators to manage this through configurations ensures flexibility and efficiency. Similarly, for old courses, I provided options for administrators to decide the time frame for system checks. For example, the system could be set to check only the courses from the last six months. If there is a need to process older courses, administrators can adjust the configurations as necessary. I hope this addresses your concerns regarding the settings. I will review all the conversations I missed and get back to you. |
|
Are you able to rebase these changes on the latest |
|
Pinging @ojnadjarm. I'd really like to get this in before I do my next release so are you able to do that rebase for me? :) |
|
Hi Mark sure, on it. |
|
Hi @mdjnelson, I wasn't using the table, so my PR modified the install.xml and the install.php was deleted. I'm not sure, if you are ok with that. Review the PR and let me know how do you want me to manage that. I'm using |
|
All the tests seems to be running just fine along with the one I added for the adhoc task after the rebase. Lmk what you think. |
Hi @ojnadjarm. It would be good if you could delete the commit in your branch that Mohamed created as that is now in MOODLE_404_STABLE. If you pull down the latest changes of MOODLE_404_STABLE then cherry-pick your commits on top from this branch (ignoring Mohameds) then pushed that then that would be great. :) |
mdjnelson
force-pushed
the
MOODLE_404_STABLE
branch
from
3cbcd9a
to
94cfd4b
Compare
ojnadjarm
force-pushed
the
DEF-2908
branch
7 times, most recently
from
187cf7e
to
1be0571
Compare
|
Hi @mdjnelson I have clean up my PR quite a bit, squash commits into 1 and delete any unnecesary changes. lmk what you think. |
|
Thanks @ojnadjarm. Ill look at this on the weekend. |
|
Hi @mdjnelson, I notice that some merge was made over the weeked, already rebased :D |
…to improve performance DEF-2908: Improving code a little bit DEF-2908: Fixing pipelines and adding adhoc task test DEF-2908: Fixing pipeline error DEF-2908: Fixing unnecesary changes DEF-2908: Fixing test
|
Ive included this in my recent updates, thanks! |
No description provided.