Search code examples
plsqloracle-apex

Using cursor for loop to get data


I want to select all employees that the contract is about to expire then if I get one employee or more then I want to send notification to their managers using email , SMS ,ESS or mobile after checking the value of them and must equal yes then I call the sending function I tried to do that using cursor for loop but I got more than one error can anyone help me?

create or replace function NOTIFICATION_SENDING(P_NOTIF_ID    NUMBER) return NUMBER
IS
    V_EMP           NUMBER;
    V_USER          NUMBER;
    V_EMAIL         VARCHAR2(1);
    V_SMS           VARCHAR2(1);
    V_ESS           VARCHAR2(1);
    V_MOBILE        VARCHAR2(1);
    CURSOR C1 IS
        SELECT EMP_ID FROM HR_EMP_CONTRACT_HEADER 
        where months_between(EDATE,sysdate) >=1
        and months_between(EDATE,sysdate)  <= 3;

    CURSOR C2 IS
        SELECT USER_ID from NL_NOTIF_EMP_HEADER
        where NOTIF_ID = P_NOTIF_ID;
    
BEGIN
    for i in C1 LOOP
        V_EMP := i.EMP_ID;
    END LOOP;
   
    IF V_EMP is not null THEN
        for i in C2 LOOP
            V_USER := i.USER_ID;
        END LOOP;
        
        IF V_USER IS NOT NULL THEN 
            SELECT EMAIL , SMS , ESS , MOBILE  
            INTO V_EMAIL , V_SMS , V_ESS , V_MOBILE
            from NL_NOTIF_EMP_HEADER
            where NOTIF_ID = P_NOTIF_ID;

            IF V_EMAIL = 'Y'  THEN
                NOTIF_EMAIL_SENDING(P_NOTIF_ID ,V_USER );
            END IF;
        
            IF  V_SMS = 'Y' THEN
                NOTIF_SMS_SENDING(P_NOTIF_ID ,V_USER );
            END IF;
            
            IF  V_ESS = 'Y' THEN
                NOTIF_ESS_SENDING(P_NOTIF_ID ,V_USER );
            END IF;
               
            IF  V_MOBILE = 'Y' THEN
                NOTIF_MOBILE_SENDING(P_NOTIF_ID ,V_USER );
            END IF;
        END IF;
    END IF;
END NOTIFICATION_SENDING;

This the code I used


Solution

  • As of your code:

    • you use cursor loops in a wrong manner, as your code affects only one (the last) value retrieved by both cursors
      • I don't know why you chose to fetch user_id in cursor and all other values in standalone select statement; seems to be unnecessary
    • that's a function, and you aren't returning anything so Oracle must be complaining about it
    • separate ifs suggest that manager wants to be (and will be) notified in many possible ways. If that's not what you wanted, consider using IF-ELSIF instead

    To me, it is unclear what nl_notif_emp_header contains nor what p_notif_id parameter represents. I think that it contains only one row for each p_notif_id value, saying that manager identified by user_id wants to get notification by ANY (can be none, one, or all four) way available.

    As you aren't returning anything, I'd say that procedure suits better.

    In my example, I'm

    • looping (unnecessary) (cur_n) through nl_notif_emp_header, just to avoid declaration of 5 local variables and use cursor variable instead
    • looping (cur_e) through employees that satisfy condition and - depending on all ways of notification - sending it to manager
      • it is unclear what notif_xxx_sending procedures do. I'd expect them to accept manager (who is to be notified) and employee info (so that manager knows which employee it is about), while you're passing p_notif_id parameter value (?!? Why?) and manager ID (that's OK). Therefore, I added yet another parameter in my example; can't tell whether that's OK or not

    So:

    create or replace procedure NOTIFICATION_SENDING (P_NOTIF_ID in number)
    is
    begin
      -- CUR_N returns only one row, but - instead of declaring bunch of local
      -- variables, I'm using a one-time cursor loop
      for cur_n in (select user_id, email, sms, ess, mobile
                    from nl_notif_emp_header
                    where notif_id = p_notif_id
                   )
      loop
        -- employees that satisfy condition
        for cur_e in (select emp_id
                      from hr_emp_contract_header
                      where months_between(edate, sysdate) between 1 and 3
                     )
        loop
          if cur_n.email = 'Y' then 
             notif_email_sending (p_notif_id, cur_n.user_id, cur_e.emp_id);
          end if;
          
          if cur_n.sms = 'Y' then
             notif_sms_sending (p_notif_id, cur_n.user_id, cur_e.emp_id);
          end if;
          
          -- etc., for all ways of notification
        end loop;
      end loop;
    end;
    /