esx-framework / esx_core

Official Repo For core resources for esx-legacy
https://documentation.esx-framework.org/
GNU General Public License v3.0
356 stars 737 forks source link

[Feature Request] - es_extended - Modifie function `ESX.CreateJob()` #1331

Closed Mesrine67 closed 2 months ago

Mesrine67 commented 4 months ago

add checks check if the job is not present in the sql.

Exemple code

Gellipapa commented 3 months ago

@Mesrine67 Hi! I like your solution but you should use the basic ESX.Jobs access as we already know all the jobs available here, no need to query the database again. There is a ESX.DoesJobExist for this only it works with grade but you can add a condition to work with job name and use that for job checking.

Mesrine67 commented 3 months ago

ok thank you for giving an answer, ESX you are the best, the only negative point I find that you should use Overextended more often for the framework it could bring a lot of advantage, example if esx_property would use Overextended entirely and not just ox_inventory I find the system would be much better

local function notifyAdmins(title, message, icon, iconType)
    local admins = ESX.GetExtendedPlayers('group', 'admin')
    for _, admin in pairs(admins) do
        admin.showAdvancedNotification(title, 'Job Management', message, icon, iconType)
    end
end

function ESX.CreateJob(name, label, grades)
    if not name or name == '' then
        notifyAdmins('Error', 'Missing argument `name(string)` while creating a job', 'CHAR_BLOCKED', 9)
        return
    end
    if not label or label == '' then
        notifyAdmins('Error', 'Missing argument `label(string)` while creating a job', 'CHAR_BLOCKED', 9)
        return
    end
    if not grades or not next(grades) then
        notifyAdmins('Error', 'Missing argument `grades(table)` while creating a job!', 'CHAR_BLOCKED', 9)
        return
    end
    local jobExists = false
    for _, grade in ipairs(grades) do
        if ESX.DoesJobExist(name, grade.grade) then
            jobExists = true
            break
        end
    end
    if jobExists then
        notifyAdmins('Warning', 'Job already exists: ' .. name, 'CHAR_BLOCKED', 9)
        return
    end
    MySQL.insert('INSERT INTO jobs (name, label) VALUES (?, ?)', {name, label}, function(jobId)
        if not jobId then
            notifyAdmins('Error', 'Failed to insert job: ' .. name, 'CHAR_BLOCKED', 9)
            return
        end
        local insertCount = 0
        for _, grade in ipairs(grades) do
            MySQL.insert('INSERT INTO job_grades (job_name, grade, name, label, salary, skin_male, skin_female) VALUES (?, ?, ?, ?, ?, ?, ?)',
            {name, grade.grade, grade.name, grade.label, grade.salary, '{}', '{}'}, function(gradeId)
                if gradeId then
                    insertCount = insertCount + 1
                    if insertCount == #grades then
                        local job = { name = name, label = label, grades = {} }
                        for _, v in pairs(grades) do
                            job.grades[tostring(v.grade)] = { job_name = name, grade = v.grade, name = v.name, label = v.label, salary = v.salary, skin_male = {}, skin_female = {} }
                        end
                        ESX.Jobs[name] = job
                        notifyAdmins('Success', 'Job created successfully: ' .. name, 'CHAR_JOB', 9)
                    end
                else
                    notifyAdmins('Error', 'Failed to insert grade for job: ' .. name, 'CHAR_BLOCKED', 9)
                end
            end)
        end
    end)
end

What do you think of the code like this? Do you have any suggestions for improvement? It would be interesting to create ESX.DoesJobGradeExist(job, grade) where grade could be a table containing multiple grades or just one grade. What are your thoughts?

Gellipapa commented 2 months ago

@Mesrine67 Hi! Sorry for the late reply, but I don't think we need such a method since we always check a grade value based on the usecases when we check if the job has a grade, so it can stay like that.

You don't need to report this to the admin to see if there is a bug on the server console as it will not be used by the players so it is enough if the developer sees it on the server console.

    local jobExists = false
    for _, grade in ipairs(grades) do
        if ESX.DoesJobExist(name, grade.grade) then
            jobExists = true
            break
        end
    end

This should be a separate local method for better readability.

INSERT INTO job_grades you should use the transaction used by ox_mysql and so you don't need the for loop and if one of them fails, none of them will be written. This would add a sense of security to the code and only one query not up to 8 if there are 8 grades.

You need the for loop because you need to compose the query string, but you don't need to call a mysql query every iteration and so this for loop logic can be exposed in a local method so you can be transparent about how you generate the output of the mysql query code.

I would suggest these code changes, thanks in advance if you make them.

Thanks.

Mesrine67 commented 2 months ago

ok what do you think of that?

local OxMysql = exports.oxmysql
function ESX.CreateJob(name, label, grades)
    local Invoke = GetInvokingResource()
    if not name or name == '' then
        print(("^5[%s]-^1[ERROR]^7 Missing argument `name`"):format(Invoke))
        return
    end
    if not label or label == '' then
        print(("^5[%s]-^1[ERROR]^7 Missing argument `label`"):format(Invoke))
        return
    end
    if not grades or not next(grades) then
        print(("^5[%s]-^1[ERROR]^7 Missing argument `grades`"):format(Invoke))
        return
    end
    local jobExists = false
    for _, grade in ipairs(grades) do
        if ESX.DoesJobExist(name, grade.grade) then
            jobExists = true
            break
        end
    end
    if jobExists then
        print(("^5[%s]-^1[ERROR]^7 Job already exists: `%s`"):format(Invoke, name))
        return
    end
    OxMysql:insert('INSERT INTO jobs (name, label) VALUES (?, ?)', {name, label}, function(jobId)
        print(jobId)
        if not jobId == 0 then
            print(("^5[%s]-^1[ERROR]^7 Failed to insert job: `%s`"):format(Invoke, name))
            return
        end
        local queries = {}
        for _, grade in ipairs(grades) do
            table.insert(queries, {
                query = 'INSERT INTO job_grades (job_name, grade, name, label, salary, skin_male, skin_female) VALUES (?, ?, ?, ?, ?, ?, ?)',
                values = {name, grade.grade, grade.name, grade.label, grade.salary, '{}', '{}'}
            })
        end
        OxMysql:transaction(queries, function(results)
            if #results == #grades then
                local job = { name = name, label = label, grades = {} }
                for _, v in pairs(grades) do
                    job.grades[tostring(v.grade)] = { job_name = name, grade = v.grade, name = v.name, label = v.label, salary = v.salary, skin_male = {}, skin_female = {} }
                end
                ESX.Jobs[name] = job
                print(("^5[%s]-^6[Success]^7 Job created successfully: `%s`"):format(Invoke, name))
            else
                print(("^5[%s]-^1[ERROR]^7 Failed to insert one or more grades for job: `%s`"):format(Invoke, name))
            end
        end, function(code)
            print(("^5[%s]-^6[INFO]^7 Transaction for job: `%s` with code: %s"):format(Invoke, name, code))
            if code == true then
                ESX.RefreshJobs()
            end
        end)
    end)
end
Gellipapa commented 2 months ago

local OxMysql = exports.oxmysql

local NOTIFY_TYPES = {
    INFO = "^5[%s]-^6[INFO]^7 %s",
    SUCCESS = "^5[%s]-^6[SUCCESS]^7 %s",
    ERROR = "^5[%s]-^1[ERROR]^7 %s"
}

local jobExists = false
local function doesJobAndGradesExist(name, grades)
    for _, grade in ipairs(grades) do
        if ESX.DoesJobExist(name, grade.grade) then
            jobExists = true
            break
        end
    end
    if jobExists then
        print(("^5[%s]-^1[ERROR]^7 Job already exists: `%s`"):format(Invoke, name))
        return
    end

    return jobExists
end

local function generateTransactionQueries(name,grades)
    local queries = {}
    for _, grade in ipairs(grades) do
        table.insert(queries, {
            query = 'INSERT INTO job_grades (job_name, grade, name, label, salary, skin_male, skin_female) VALUES (?, ?, ?, ?, ?, ?, ?)',
            values = {name, grade.grade, grade.name, grade.label, grade.salary, '{}', '{}'}
        })
    end

    return queries
end

local function generateNewJobTable(name, label, grades)
    local job = { name = name, label = label, grades = {} }
    for _, v in pairs(grades) do
        job.grades[tostring(v.grade)] = { job_name = name, grade = v.grade, name = v.name, label = v.label, salary = v.salary, skin_male = {}, skin_female = {} }
    end

    return job
end

local function notify(notifyType,resourceName,message,...)

    if not NOTIFY_TYPES[notifyType] then
        return print(NOTIFY_TYPES.INFO:format(resourceName,message,...))
    end

    return print(NOTIFY_TYPES[notifyType]:format(resourceName,message,...))
end

function ESX.CreateJob(name, label, grades)
    local currentResourceName = GetInvokingResource()

    if not name or name == '' then
        notify("ERROR",currentResourceName, 'Missing argument `name`')
        return
    end
    if not label or label == '' then
        notify("ERROR",currentResourceName, 'Missing argument `label`')
        return
    end
    if not grades or not next(grades) then
        notify("ERROR",currentResourceName, 'Missing argument `grades`')
        return
    end

    local currentJobExist = doesJobAndGradesExist(name, grades)

    if currentJobExist then
        notify("ERROR",currentResourceName, 'Job already exists: `%s`', name)
        return
    end

    OxMysql:insert('INSERT INTO jobs (name, label) VALUES (?, ?)', {name, label}, function(jobId)
        if not jobId == 0 then
            notify("ERROR",currentResourceName, 'Failed to insert job: `%s`', name)
            return
        end

        local queries = generateTransactionQueries()

        OxMysql:transaction(queries, function(results)
            if #results == #grades then
                ESX.Jobs[name] = generateNewJobTable(name,label,grades)
                notify("SUCCESS",currentResourceName, 'Job created successfully: `%s`', name)
            else
                notify("ERROR",currentResourceName, 'Failed to insert one or more grades for job: `%s`', name)
            end
        end, function(code)
            if not code then
                return
            end
            notify("INFO",currentResourceName, 'Transaction for job: `%s` with code: %s', name, code)
            ESX.RefreshJobs()
        end)
    end)
end

You would have to do something like this in a PR and then it would be mergable, you would have to write it in a job.lua under the module so you can manage it uniformly.

Gellipapa commented 2 months ago

1355 Implemented new logic.